chore: Replace Meteor.userId() for client code
#36911
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughReplace direct Meteor global user access with a Zustand-backed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Accounts as Accounts.connection
participant Store as userIdStore
participant Watch as watchUserId/watchUser
participant Helpers as getUserId/getUser
participant Client as Client Modules
Accounts->>Store: setState(userId) on login/logout
Client->>Helpers: call getUserId()/getUser()
Helpers-->>Client: return userId or user (or undefined)
Client->>Watch: subscribe to watchUserId()/watchUser() (reactive)
Watch-->>Client: emit updates when store changes
sequenceDiagram
autonumber
participant Thread as ThreadTitle
participant Msg as mainMessage
participant Conf as Settings/User
participant Emoji as emojiParser
participant Parser as MentionsParser
Thread->>Msg: read msg / attachments
Thread->>Conf: read patterns & useRealName
alt has mentions or channels
Thread->>Emoji: emojiParser(msg)
Thread->>Parser: MentionsParser.parse(html)
Parser-->>Thread: parsed html
else plain msg
Thread-->>Thread: escapeHTML + filterMarkdown -> text
else attachments
Thread-->>Thread: escapeHTML(title/description)
end
Thread-->>UI: render html via dangerouslySetInnerHTML
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #36911 +/- ##
===========================================
+ Coverage 67.37% 67.39% +0.01%
===========================================
Files 3330 3331 +1
Lines 113482 112817 -665
Branches 20601 20608 +7
===========================================
- Hits 76464 76037 -427
+ Misses 34408 34162 -246
- Partials 2610 2618 +8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (2)
256-266: Bug: comparator usesa._idfor both sides.
idBshould start fromb._id. Current code biases sorting and can misorder results.- const emojiSort = (recents: string[]) => (a: { _id: string }, b: { _id: string }) => { - let idA = a._id; - let idB = a._id; + const emojiSort = (recents: string[]) => (a: { _id: string }, b: { _id: string }) => { + let idA = a._id; + let idB = b._id;
227-231: Bug: picking emoji data withcollection[key]instead ofcollection[_id].This returns the same entry for all items. Select by the iterated key.
- return Object.keys(collection) - .map((_id) => { - const data = collection[key]; - return { _id, data }; - }) + return Object.keys(collection) + .map((_id) => { + const data = collection[_id]; + return { _id, data }; + })Also applies to: 285-288
apps/meteor/client/apps/RealAppsEngineUIHost.ts (1)
62-68: Avoid non-null assertions; guard missing user to prevent runtime crash.getUser() can be undefined during init or transient auth states; current code would throw.
Apply:
- const { username, _id } = getUser()!; - - return { - id: _id, - username: username!, - avatarUrl: this.getUserAvatarUrl(username!) || '', - }; + const user = getUser(); + if (!user?.username || !user?._id) { + throw new Error('User not found'); + } + const { username, _id } = user; + return { + id: _id, + username, + avatarUrl: this.getUserAvatarUrl(username), + };apps/meteor/client/lib/mutationEffects/starredMessage.ts (1)
7-16: Avoid writing stars with undefined _id; remove non-null assertion.getUserId() can be undefined; current code may persist
{ _id: undefined }inrecord.starred. Guard and no-op when not logged in.Apply:
- const uid = getUserId()!; + const uid = getUserId(); + if (!uid) { + return; + }Optional dedupe to prevent multiple entries for the same user:
- starred: [...(record.starred ?? []), { _id: uid }], + starred: (record.starred ?? []).some((s) => s._id === uid) + ? record.starred + : [...(record.starred ?? []), { _id: uid }],apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
272-287: Fix crash when userId is unavailable (instance may be undefined)If
getUserId()returns undefined, the instance is not created and the subsequent access to.keyIDthrows. Guard and return early.- const userId = getUserId(); - if (!this.instancesByRoomId[rid] && userId) { - this.instancesByRoomId[rid] = new E2ERoom(userId, room); - } - - // When the key was already set and is changed via an update, we update the room instance - if ( - this.instancesByRoomId[rid].keyID !== undefined && - room.e2eKeyId !== undefined && - this.instancesByRoomId[rid].keyID !== room.e2eKeyId - ) { - // KeyID was changed, update instance with new keyID and put room in waiting keys status - this.instancesByRoomId[rid].onRoomKeyReset(room.e2eKeyId); - } - - return this.instancesByRoomId[rid]; + const userId = getUserId(); + if (!this.instancesByRoomId[rid] && userId) { + this.instancesByRoomId[rid] = new E2ERoom(userId, room); + } + const instance = this.instancesByRoomId[rid]; + if (!instance) { + return null; + } + // When the key was already set and is changed via an update, we update the room instance + if (instance.keyID !== undefined && room.e2eKeyId !== undefined && instance.keyID !== room.e2eKeyId) { + // KeyID was changed, update instance with new keyID and put room in waiting keys status + instance.onRoomKeyReset(room.e2eKeyId); + } + return instance;apps/meteor/app/authorization/client/hasPermission.ts (1)
12-12: Use IRole IDs for scopedRoles (not IPermission IDs).
scopedRolesrepresents role IDs and is compared againstroleIdfrompermission.roles. Typing it asIPermission['_id'][]is misleading and risks future misuse.Apply:
-import type { IUser, IPermission } from '@rocket.chat/core-typings'; +import type { IUser, IPermission, IRole } from '@rocket.chat/core-typings'; @@ - (permissionIds: IPermission['_id'][], scope: string | undefined, userId: IUser['_id'], scopedRoles?: IPermission['_id'][]): boolean => { + (permissionIds: IPermission['_id'][], scope: string | undefined, userId: IUser['_id'], scopedRoles?: IRole['_id'][]): boolean => { @@ - predicate: ( + predicate: ( permissionIds: IPermission['_id'][], scope: string | undefined, userId: IUser['_id'], - scopedRoles?: IPermission['_id'][], + scopedRoles?: IRole['_id'][], ) => boolean, - userId?: IUser['_id'], - scopedRoles?: IPermission['_id'][], + userId?: IUser['_id'], + scopedRoles?: IRole['_id'][], @@ export const hasAllPermission = ( permissions: IPermission['_id'] | IPermission['_id'][], scope?: string, - scopedRoles?: IPermission['_id'][], + scopedRoles?: IRole['_id'][], ): boolean => validatePermissions(permissions, scope, all, undefined, scopedRoles);Also applies to: 44-49, 50-52, 66-70
apps/meteor/app/autotranslate/client/lib/autotranslate.ts (2)
98-104: Make the permission gate reactive (role/permission flips may be missed).The autorun depends on settings and userId, but not on permission state. If a user’s roles/permissions load or change after login, c.stop() may never trigger. Add a reactive dependency that changes when permissions/roles for the current user change (e.g., also watchUser() to bind to role changes, or a dedicated permission watcher), then evaluate hasPermission.
Apply this minimal change to bind to user record changes:
Tracker.autorun(async (c) => { const uid = watchUserId(); + // bind to user changes so permission recalculations retrigger this autorun + const _user = watchUser(); + void _user; if (!settings.watch('AutoTranslate_Enabled') || !uid || !hasPermission('auto-translate')) { return; } c.stop(); try {
83-87: Propagate autoTranslateShowInverse in recursive call.The recursive translateAttachments call drops the autoTranslateShowInverse flag, changing behavior for nested attachments.
- attachment.attachments = this.translateAttachments(attachment.attachments, language); + attachment.attachments = this.translateAttachments(attachment.attachments, language, autoTranslateShowInverse);apps/meteor/client/providers/UserProvider/UserProvider.tsx (1)
141-156: Capture user before logout and emit to ensure cleanup runs.Emit the pre-logout user so the onLogout handler can always perform cleanup, regardless of store timing.
const contextValue = useMemo( (): ContextType<typeof UserContext> => ({ userId, user, queryPreference: createReactiveSubscriptionFactory( <T,>(key: string, defaultValue?: T) => getUserPreference(userId, key, defaultValue) as T, ), querySubscription, queryRoom, querySubscriptions, - logout: async () => Meteor.logout(), + logout: async () => { + const uid = userIdStore.getState(); + const preLogoutUser = uid ? Users.state.get(uid) : undefined; + return Meteor.logout(() => { + ee.emit('logout', preLogoutUser); + }); + }, onLogout: (cb) => { return ee.on('logout', cb); }, }), [userId, user, querySubscription, querySubscriptions], );
🧹 Nitpick comments (36)
apps/meteor/client/meteor/overrides/unstoreLoginToken.ts (1)
6-9: Harden logout path: preserve return, ensure cache clear in finally, and don’t let cache invalidation break logout.Use try/finally so caches are cleared even if
_unstoreLoginTokenthrows, and return the original function’s result to preserve semantics. Also add a minimal type for args.-Accounts._unstoreLoginToken = (...args) => { - _unstoreLoginToken.apply(Accounts, args); - CachedStoresManager.clearAllCachesOnLogout(); -}; +Accounts._unstoreLoginToken = (...args: Parameters<typeof _unstoreLoginToken>) => { + try { + return _unstoreLoginToken.apply(Accounts, args); + } finally { + // Cache invalidation should be non-blocking for logout. + try { + CachedStoresManager.clearAllCachesOnLogout(); + } catch {} + } +};apps/meteor/client/hooks/useIdleConnection.ts (4)
8-11: Avoid double subscriptions to connection status.Call
useConnectionStatus()once to getstatus,disconnect, andreconnect.-const { status } = useConnectionStatus(); -const allowAnonymousRead = useSetting('Accounts_AllowAnonymousRead'); -const { disconnect: disconnectServer, reconnect: reconnectServer } = useConnectionStatus(); +const { status, disconnect: disconnectServer, reconnect: reconnectServer } = useConnectionStatus(); +const allowAnonymousRead = useSetting('Accounts_AllowAnonymousRead');
9-16: Type the setting as boolean and simplify the condition.
useSetting<boolean>(...) ?? falseclarifies intent and removes!== truechecks.-const allowAnonymousRead = useSetting('Accounts_AllowAnonymousRead'); +const allowAnonymousRead = useSetting<boolean>('Accounts_AllowAnonymousRead') ?? false; … -const disconnect = useEffectEvent(() => { - if (status !== 'offline') { - if (!uid && allowAnonymousRead !== true) { - disconnectServer(); - } - } -}); +const disconnect = useEffectEvent(() => { + if (status !== 'offline' && !uid && !allowAnonymousRead) { + disconnectServer(); + } +});
26-26: Rename event id to match hookReplace the stale id 'useLoginPresence' with 'useIdleConnection' in apps/meteor/client/hooks/useIdleConnection.ts:26 — only this occurrence found.
-useIdleActiveEvents({ id: 'useLoginPresence', time: 60 * 1000 }, disconnect, reconnect); +useIdleActiveEvents({ id: 'useIdleConnection', time: 60 * 1000 }, disconnect, reconnect);
7-7: Signature change is safe — makeuidoptional if you prefer.Repo search: only call site is apps/meteor/client/providers/UserProvider/UserProvider.tsx:89 (calls useIdleConnection(userId)); userId comes from userIdStore (IUser['_id'] | undefined). No callers pass
null.Suggested diff:
-export const useIdleConnection = (uid: IUser['_id'] | undefined) => { +export const useIdleConnection = (uid?: IUser['_id']) => {apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (4)
25-27: Check emptiness, not existence, of mentions/channelsEmpty arrays are truthy; this path will run the heavier parser unnecessarily.
- if (!message.channels && !message.mentions) { + if (!message.channels?.length && !message.mentions?.length) {
36-37: Name collision/readability: avoid reusing “html”Clarify intent by renaming the intermediate variable.
- const html = emojiParser(filteredMessage); - return instance.parse({ ...message, msg: filteredMessage, html }).html ?? ''; + const htmlWithEmojis = emojiParser(filteredMessage); + return instance.parse({ ...message, msg: filteredMessage, html: htmlWithEmojis }).html ?? '';
17-18: Type the settings for stronger guaranteesEnsure correct types and fallbacks without relying on default param overloads.
- const pattern = useSetting('UTF8_User_Names_Validation', '[0-9a-zA-Z-_.]+'); - const useRealName = useSetting('UI_Use_Real_Name', false); + const pattern = useSetting<string>('UTF8_User_Names_Validation') ?? '[0-9a-zA-Z-_.]+'; + const useRealName = useSetting<boolean>('UI_Use_Real_Name') ?? false;
21-22: Avoid unnecessary clone of mainMessageYou don’t mutate it; read directly to reduce allocations.
- const message = { ...mainMessage }; + const message = mainMessage;apps/meteor/client/views/room/MemberListRouter.tsx (1)
9-19: Prefer Array.find over filter().shift(); add empty-uids guard.
Avoids an intermediate array and makes intent clearer.-const getUid = (room: IRoom, ownUserId: string | undefined) => { - if (room.uids?.length === 1) { - return room.uids[0]; - } - - const uid = room.uids?.filter((uid) => uid !== ownUserId).shift(); - - // Self DMs used to be created with the userId duplicated. - // Sometimes rooms can have 2 equal uids, but it's a self DM. - return uid || room.uids?.[0]; -}; +const getUid = (room: IRoom, ownUserId: string | undefined) => { + if (room.uids?.length === 1) { + return room.uids[0]; + } + + if (!room.uids?.length) { + return undefined; + } + + const uid = room.uids.find((id) => id !== ownUserId); + + // Self DMs used to be created with the userId duplicated. + // Sometimes rooms can have 2 equal uids, but it's a self DM. + return uid || room.uids[0]; +};apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
13-49: Guard is correct; add parseInt radix to avoid implicit base.
Keeps behavior deterministic and silences linters.- useMemo(() => parseInt(`${getConfig('discussionListSize', 10)}`), []), + useMemo(() => parseInt(`${getConfig('discussionListSize', 10)}`, 10), []),Optional: if callers are expected to pass a defined uid, consider tightening the signature to
uid: IUser['_id']and letting call sites gate usage.apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
18-55: Type update aligns with uid:=undefined; add parseInt radix and simplify effect.
Minor polish; no behavioral change.- useComponentDidUpdate(() => { - options && reload(); - }, [options, reload]); + useComponentDidUpdate(() => { + reload(); + }, [options, reload]); - useMemo(() => parseInt(`${getConfig('historyMessageListSize', 10)}`), []), + useMemo(() => parseInt(`${getConfig('historyMessageListSize', 10)}`, 10), []),apps/meteor/client/lib/utils/getUidDirectMessage.ts (1)
6-14: Simplify default and prefer find() over filter()[0].Removes a redundant
?? undefinedand makes the “other participant” selection clearer.-export const getUidDirectMessage = (rid: IRoom['_id'], uid: IUser['_id'] | undefined = getUserId() ?? undefined): string | undefined => { +export const getUidDirectMessage = (rid: IRoom['_id'], uid: IUser['_id'] | undefined = getUserId()): string | undefined => { const room = Rooms.state.get(rid); if (!room || room.t !== 'd' || !room.uids || room.uids.length > 2) { return undefined; } - return room.uids.filter((_uid) => _uid !== uid)[0]; + return room.uids.find((_uid) => _uid !== uid); }Note: If DMs are guaranteed to have exactly two members, consider tightening the length check to
!== 2.apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts (1)
142-142: Preserve server default forshowThreadsInMainChannel.Passing a hard default
falseprevents falling back toAccounts_Default_User_Preferences_showThreadsInMainChannel. Prefer typing the return and omitting the default so the setting default applies.-const showThreadsInMainChannel = getUserPreference(getUserId(), 'showThreadsInMainChannel', false); +const showThreadsInMainChannel = getUserPreference<boolean>(getUserId(), 'showThreadsInMainChannel');apps/meteor/client/lib/cachedStores/CachedStore.ts (1)
78-81: (Optional) Fail fast if userId is missing fornotify-user.If you want stronger guarantees, assert the presence of userId when computing the event. Keeps subtle bugs from slipping in during refactors.
- if (this.eventType === 'notify-user') { - return `${getUserId()}/${this.name}-changed`; - } + if (this.eventType === 'notify-user') { + const uid = getUserId(); + if (!uid) { + // Defensive: this.store.listen() should only init after login + throw new Error('notify-user subscriptions require a logged-in user'); + } + return `${uid}/${this.name}-changed`; + }apps/meteor/client/lib/chats/flows/processTooLongMessage.ts (1)
29-31: Sanitize and stabilize attachment filename.New Date().toString() yields spaces/colons; use ISO and sanitize username for cross-platform safety.
- const fileName = `${getUser()?.username ?? 'anonymous'} - ${new Date()}.txt`; // TODO: proper naming and formatting + const username = (getUser()?.username ?? 'anonymous').replace(/[^\w.-]/g, '_'); + const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + const fileName = `long-message_${username}_${timestamp}.txt`;apps/meteor/client/startup/incomingMessages.ts (2)
13-18: Guard against undefined userId and avoid recomputing.Although wrapped by onLoggedIn, a defensive guard avoids accidental subscriptions if userId is ever undefined and keeps cleanup semantics intact.
Apply:
- return sdk.stream('notify-user', [`${getUserId()}/message`], (msg: IMessage) => { + const uid = getUserId(); + if (!uid) return () => undefined; + return sdk.stream('notify-user', [`${uid}/message`], (msg: IMessage) => {
21-34: Same guard for subscriptions-changed stream.Apply:
- return sdk.stream('notify-user', [`${getUserId()}/subscriptions-changed`], (_action, sub) => { + const uid = getUserId(); + if (!uid) return () => undefined; + return sdk.stream('notify-user', [`${uid}/subscriptions-changed`], (_action, sub) => {apps/meteor/client/lib/getPermaLink.ts (1)
32-33: Nit: compute userId once to avoid repeated store reads inside the predicate.Apply:
- const subData = Subscriptions.state.find((record) => record.rid === roomData._id && record.u._id === getUserId()); + const uid = getUserId(); + const subData = Subscriptions.state.find((record) => record.rid === roomData._id && record.u._id === uid);apps/meteor/app/otr/client/OTRRoom.ts (1)
105-114: Early-exit sooner on refresh when no user to avoid unnecessary work.Right now keypair generation and publish happen before the no-user return. Consider checking for user (and returning) before generating keys/publishing in refresh flows if that matches desired semantics.
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts (1)
24-26: Optional: omituidfrom params when undefined.Slightly cleaner payload; avoids serializing
uid: undefinedif the method does not expect the key when absent.Apply:
- const result = await searchMessages(searchText, { uid, rid: room._id }, { limit, searchAll: globalSearch }); + const params = uid ? { uid, rid: room._id } : { rid: room._id }; + const result = await searchMessages(searchText, params, { limit, searchAll: globalSearch });packages/ui-contexts/src/hooks/useUserId.ts (1)
5-5: Keep explicit return type for public hook API.Prevents accidental type drift and documents the undefined semantics.
-export const useUserId = () => useContext(UserContext).userId; +export const useUserId = (): string | undefined => useContext(UserContext).userId;apps/meteor/client/startup/iframeCommands.ts (1)
80-88: Ensure navigation after logout even when no user object is present.Currently skips router.navigate('/home') if user is falsy; likely unintended behavior change.
- const user = getUser(); - Meteor.logout(() => { - if (!user) return; - - void afterLogoutCleanUpCallback.run(user); - sdk.call('logoutCleanUp', user as unknown as IUser); - return router.navigate('/home'); - }); + const user = getUser(); + Meteor.logout(() => { + if (user) { + void afterLogoutCleanUpCallback.run(user); + sdk.call('logoutCleanUp', user as unknown as IUser); + } + return router.navigate('/home'); + });apps/meteor/client/meteor/overrides/ddpOverREST.ts (1)
61-76: Update comment to reflect getUserId/userId store (no longer Meteor.userId).Avoid confusion for future maintainers.
-// 5. Accounts.onLogin callbacks are triggered, Meteor.userId is set +// 5. Accounts.onLogin callbacks are triggered, the userId store is set (getUserId() returns a value)apps/meteor/client/lib/chats/data.ts (1)
198-207: Avoid double-reading userId; use the local uid for consistency.Prevents mismatch if identity changes mid-execution and saves a call.
- const deleteOwnAllowed = hasPermission('delete-own-message'); - const deleteAllowed = deleteAnyAllowed || (deleteOwnAllowed && message?.u && message.u._id === getUserId()); + const deleteOwnAllowed = hasPermission('delete-own-message'); + const deleteAllowed = deleteAnyAllowed || (deleteOwnAllowed && message?.u && message.u._id === uid);apps/meteor/app/lib/client/methods/sendMessage.ts (1)
29-33: Optional: avoid forcing empty name fieldSetting
name: user.name || ''persists empty strings. Consider omitting the field when absent to reduce payload churn.- name: user.name || '', + ...(user.name ? { name: user.name } : {}),apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
855-857: Minor: avoid repeated getUserId calls inside predicateCache
selfIdonce for the predicate closure to avoid micro re-reads.- const predicate = (record: IRoom) => - Boolean('usersWaitingForE2EKeys' in record && record.usersWaitingForE2EKeys?.every((user) => user.userId !== getUserId())); + const selfId = getUserId(); + const predicate = (record: IRoom) => + Boolean('usersWaitingForE2EKeys' in record && record.usersWaitingForE2EKeys?.every((user) => user.userId !== selfId));apps/meteor/client/providers/AuthorizationProvider.tsx (1)
29-37: Optional: keep stable references for non-user-bound members
getRolesandsubscribeToRolesdon’t depend onuserId; you can move them outside the memo to reduce needless re-instantiation.- const contextValue = useMemo( - () => ({ + const getRoles = () => Roles.state.records; + const subscribeToRoles = (callback: () => void) => Roles.use.subscribe(callback); + const contextValue = useMemo( + () => ({ queryPermission: createReactiveSubscriptionFactory((permission, scope, scopeRoles) => hasPermission(permission, scope, scopeRoles)), queryAtLeastOnePermission: createReactiveSubscriptionFactory((permissions, scope) => hasAtLeastOnePermission(permissions, scope)), queryAllPermissions: createReactiveSubscriptionFactory((permissions, scope) => hasAllPermission(permissions, scope)), queryRole: createReactiveSubscriptionFactory((role, scope?) => !!userId && hasRole(userId, role, scope)), - getRoles: () => Roles.state.records, - subscribeToRoles: (callback: () => void) => Roles.use.subscribe(callback), + getRoles, + subscribeToRoles, }), [userId], );apps/meteor/app/authorization/client/hasPermission.ts (3)
53-53: Remove redundant nullish coalescing.
?? undefinedis a no-op. Use??=for clarity.-userId = userId ?? watchUserId() ?? undefined; +userId ??= watchUserId();
15-18: Avoid binding Array prototype; call directly for clarity.Binding
Array.prototype.every/someto use as methods is non-idiomatic.-const checkEachPermission = quantifier.bind(permissionIds); +const checkEachPermission = (cb: (permissionId: IPermission['_id']) => boolean) => + quantifier.call(permissionIds, cb);
72-74: Expose scopedRoles on hasAtLeastOnePermission for parity.If
scopedRolesis intended (as onhasAllPermission), add it here too. If intentionally omitted, ignore.-export const hasAtLeastOnePermission = (permissions: IPermission['_id'] | IPermission['_id'][], scope?: string): boolean => - validatePermissions(permissions, scope, atLeastOne); +export const hasAtLeastOnePermission = ( + permissions: IPermission['_id'] | IPermission['_id'][], + scope?: string, + scopedRoles?: IRole['_id'][], +): boolean => validatePermissions(permissions, scope, atLeastOne, undefined, scopedRoles);apps/meteor/client/lib/user.ts (2)
6-11: Keep the store private; expose a subscribe helper.You warn not to consume the store directly but other modules import it. Provide a wrapper to keep the contract stable.
export const userIdStore = create<IUser['_id'] | undefined>(() => undefined); -export const getUserId = () => userIdStore.getState(); +export const getUserId = (): IUser['_id'] | undefined => userIdStore.getState(); + +export const subscribeUserId = ( + listener: (id: IUser['_id'] | undefined, prev: IUser['_id'] | undefined) => void, +) => userIdStore.subscribe(listener);
13-17: Add explicit return type for getUser.Minor clarity/readability improvement.
-export const getUser = () => { +export const getUser = (): IUser | undefined => { const userId = getUserId(); if (!userId) return undefined; return Users.state.get(userId); };apps/meteor/client/startup/accounts.ts (2)
8-9: Honor “do not consume store directly” guidance.Prefer importing a wrapper (e.g.,
subscribeUserId) rather than the raw store.
46-52: Throwing inside callback may be swallowed.The thrown
Meteor.Errorin a callback is unlikely to be observed upstream. Consider logging and early-returning after toast, or rejecting a Promise if a caller awaits.- if (error) { - dispatchToastMessage({ type: 'error', message: error }); - throw new Meteor.Error('verify-email', 'E-mail not verified'); - } else { + if (error) { + dispatchToastMessage({ type: 'error', message: error }); + // Avoid throwing here; it won't be caught by a caller. + return; + } else { void sdk.call('afterVerifyEmail'); dispatchToastMessage({ type: 'success', message: t('Email_verified') }); }apps/meteor/client/meteor/overrides/userAndUsers.ts (1)
10-12: Avoid redundant store updates.Guard setState to prevent unnecessary updates/listener churn when the userId value hasn’t changed.
Tracker.autorun(() => { const userId = Accounts.connection.userId() ?? undefined; - userIdStore.setState(userId); + if (userIdStore.getState() !== userId) { + userIdStore.setState(userId); + } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (75)
apps/meteor/app/authorization/client/hasPermission.ts(3 hunks)apps/meteor/app/autotranslate/client/lib/autotranslate.ts(4 hunks)apps/meteor/app/lib/client/methods/sendMessage.ts(1 hunks)apps/meteor/app/otr/client/OTRRoom.ts(2 hunks)apps/meteor/app/reactions/client/methods/setReaction.ts(1 hunks)apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts(2 hunks)apps/meteor/app/ui/client/lib/ChatMessages.ts(2 hunks)apps/meteor/app/ui/client/lib/UserAction.ts(3 hunks)apps/meteor/app/utils/client/lib/getUserPreference.ts(2 hunks)apps/meteor/app/webrtc/client/WebRTCClass.ts(2 hunks)apps/meteor/client/apps/RealAppsEngineUIHost.ts(2 hunks)apps/meteor/client/cachedStores/PermissionsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/RoomsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/SubscriptionsCachedStore.ts(1 hunks)apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts(1 hunks)apps/meteor/client/hooks/useIdleConnection.ts(1 hunks)apps/meteor/client/lib/2fa/process2faReturn.ts(3 hunks)apps/meteor/client/lib/cachedStores/CachedStore.ts(2 hunks)apps/meteor/client/lib/cachedStores/index.ts(0 hunks)apps/meteor/client/lib/chats/ChatAPI.ts(1 hunks)apps/meteor/client/lib/chats/data.ts(6 hunks)apps/meteor/client/lib/chats/flows/processTooLongMessage.ts(2 hunks)apps/meteor/client/lib/chats/readStateManager.ts(2 hunks)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts(4 hunks)apps/meteor/client/lib/getPermaLink.ts(2 hunks)apps/meteor/client/lib/loggedIn.ts(1 hunks)apps/meteor/client/lib/mutationEffects/room.ts(1 hunks)apps/meteor/client/lib/mutationEffects/starredMessage.ts(1 hunks)apps/meteor/client/lib/normalizeThreadTitle.ts(0 hunks)apps/meteor/client/lib/user.ts(1 hunks)apps/meteor/client/lib/utils/getUidDirectMessage.ts(1 hunks)apps/meteor/client/lib/utils/timeAgo.ts(1 hunks)apps/meteor/client/meteor/minimongo/Cursor.ts(1 hunks)apps/meteor/client/meteor/minimongo/DiffSequence.ts(1 hunks)apps/meteor/client/meteor/minimongo/LocalCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/ObserveHandle.ts(1 hunks)apps/meteor/client/meteor/minimongo/common.ts(1 hunks)apps/meteor/client/meteor/overrides/ddpOverREST.ts(2 hunks)apps/meteor/client/meteor/overrides/unstoreLoginToken.ts(1 hunks)apps/meteor/client/meteor/overrides/userAndUsers.ts(1 hunks)apps/meteor/client/meteor/user.ts(1 hunks)apps/meteor/client/providers/AuthorizationProvider.tsx(2 hunks)apps/meteor/client/providers/SettingsProvider.tsx(1 hunks)apps/meteor/client/providers/UserProvider/UserProvider.tsx(4 hunks)apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts(1 hunks)apps/meteor/client/startup/accounts.ts(1 hunks)apps/meteor/client/startup/iframeCommands.ts(2 hunks)apps/meteor/client/startup/incomingMessages.ts(2 hunks)apps/meteor/client/startup/roles.ts(2 hunks)apps/meteor/client/startup/startup.ts(1 hunks)apps/meteor/client/stores/Messages.ts(1 hunks)apps/meteor/client/stores/Permissions.ts(1 hunks)apps/meteor/client/stores/Roles.ts(1 hunks)apps/meteor/client/stores/Rooms.ts(1 hunks)apps/meteor/client/stores/Settings.ts(1 hunks)apps/meteor/client/stores/Subscriptions.ts(1 hunks)apps/meteor/client/stores/Users.ts(1 hunks)apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts(1 hunks)apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts(1 hunks)apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts(1 hunks)apps/meteor/client/views/room/MemberListRouter.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts(1 hunks)apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts(1 hunks)apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts(1 hunks)apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx(1 hunks)apps/meteor/definition/externals/meteor/accounts-base.d.ts(1 hunks)packages/mock-providers/src/MockedAppRootBuilder.tsx(2 hunks)packages/ui-contexts/src/UserContext.ts(2 hunks)packages/ui-contexts/src/hooks/useUserId.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/lib/normalizeThreadTitle.ts
- apps/meteor/client/lib/cachedStores/index.ts
🧰 Additional context used
🧬 Code graph analysis (46)
apps/meteor/app/reactions/client/methods/setReaction.ts (1)
apps/meteor/client/lib/user.ts (2)
getUserId(11-11)getUser(13-17)
apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts (2)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
getUserPreference(40-47)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/chats/flows/processTooLongMessage.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/client/lib/mutationEffects/starredMessage.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/mutationEffects/room.ts (1)
apps/meteor/app/channel-settings/server/methods/saveRoomSettings.ts (1)
favorite(314-316)
apps/meteor/client/meteor/user.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)
apps/meteor/client/lib/chats/readStateManager.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/cachedStores/CachedStore.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/lib/chats/data.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/getPermaLink.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (2)
apps/meteor/app/livechat/server/methods/getFirstRoomMessage.ts (1)
uid(17-35)packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/app/ui/client/lib/ChatMessages.ts (3)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/IUser.ts (1)
IUser(186-252)packages/ui-contexts/src/index.ts (1)
IActionManager(18-18)
apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/client/lib/utils/timeAgo.ts (2)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
getUserPreference(40-47)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/2fa/process2faReturn.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/app/otr/client/OTRRoom.ts (2)
apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (2)
apps/meteor/app/livechat/server/methods/getFirstRoomMessage.ts (1)
uid(17-35)packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx (1)
packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)
packages/ui-contexts/src/hooks/useUserId.ts (1)
packages/ui-contexts/src/UserContext.ts (1)
UserContext(54-63)
apps/meteor/client/startup/roles.ts (1)
apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/app/webrtc/client/WebRTCClass.ts (3)
apps/meteor/client/lib/settings/settings.ts (1)
settings(68-68)apps/meteor/client/lib/settings/index.ts (1)
settings(3-3)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/meteor/overrides/ddpOverREST.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/app/lib/client/methods/sendMessage.ts (1)
apps/meteor/client/lib/user.ts (2)
getUserId(11-11)getUser(13-17)
apps/meteor/client/lib/loggedIn.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/startup/incomingMessages.ts (2)
apps/meteor/client/lib/loggedIn.ts (1)
onLoggedIn(23-40)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/MemberListRouter.tsx (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/lib/user.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/client/apps/RealAppsEngineUIHost.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/client/lib/utils/getUidDirectMessage.ts (3)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/IUser.ts (1)
IUser(186-252)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/hooks/useIdleConnection.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/client/providers/AuthorizationProvider.tsx (3)
apps/meteor/client/lib/createReactiveSubscriptionFactory.ts (1)
createReactiveSubscriptionFactory(7-45)apps/meteor/app/authorization/client/hasPermission.ts (3)
hasPermission(81-81)hasAtLeastOnePermission(72-73)hasAllPermission(66-70)apps/meteor/app/authorization/client/hasRole.ts (1)
hasRole(6-21)
apps/meteor/client/startup/iframeCommands.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts (1)
packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)
apps/meteor/client/startup/accounts.ts (5)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/cachedStores/index.ts (2)
SubscriptionsCachedStore(5-5)PublicSettingsCachedStore(3-3)apps/meteor/client/lib/userData.ts (1)
useUserDataSyncReady(8-8)apps/meteor/client/lib/toast.ts (1)
dispatchToastMessage(22-25)apps/meteor/app/utils/client/lib/SDKClient.ts (1)
sdk(271-271)
apps/meteor/client/startup/startup.ts (1)
apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/app/ui/client/lib/UserAction.ts (1)
apps/meteor/client/lib/user.ts (2)
getUser(13-17)getUserId(11-11)
apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (2)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)apps/meteor/client/lib/e2ee/helper.ts (2)
deriveKey(112-117)toArrayBuffer(12-27)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (3)
packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(274-278)packages/ui-contexts/src/index.ts (2)
useUser(80-80)useSetting(69-69)apps/meteor/app/emoji/client/emojiParser.ts (1)
emojiParser(7-75)
apps/meteor/app/autotranslate/client/lib/autotranslate.ts (2)
apps/meteor/client/meteor/user.ts (2)
watchUser(9-13)watchUserId(7-7)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/meteor/overrides/userAndUsers.ts (2)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/meteor/user.ts (2)
watchUserId(7-7)watchUser(9-13)
apps/meteor/client/providers/UserProvider/UserProvider.tsx (4)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)apps/meteor/client/stores/Users.ts (1)
Users(11-13)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts (1)
state(82-84)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (2)
apps/meteor/app/livechat/server/methods/getFirstRoomMessage.ts (1)
uid(17-35)packages/core-typings/src/IUser.ts (1)
IUser(186-252)
apps/meteor/app/authorization/client/hasPermission.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-252)
🪛 ast-grep (0.38.6)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
[warning] 55-55: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
| const emitActivities = debounce(async (rid: string, extras: IExtras): Promise<void> => { | ||
| const activities = roomActivities.get(extras?.tmid || rid) || new Set(); | ||
| sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, shownName(Meteor.user() as unknown as IUser), [...activities], extras]); | ||
| sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, shownName(getUser()), [...activities], extras]); | ||
| }, 500); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against undefined username in activity broadcast
shownName(getUser()) can be undefined (e.g., before user record loads). Avoid emitting an event with an undefined username.
- sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, shownName(getUser()), [...activities], extras]);
+ const username = shownName(getUser());
+ if (!username) return;
+ sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, username, [...activities], extras]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const emitActivities = debounce(async (rid: string, extras: IExtras): Promise<void> => { | |
| const activities = roomActivities.get(extras?.tmid || rid) || new Set(); | |
| sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, shownName(Meteor.user() as unknown as IUser), [...activities], extras]); | |
| sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, shownName(getUser()), [...activities], extras]); | |
| }, 500); | |
| const emitActivities = debounce(async (rid: string, extras: IExtras): Promise<void> => { | |
| const activities = roomActivities.get(extras?.tmid || rid) || new Set(); | |
| const username = shownName(getUser()); | |
| if (!username) return; | |
| sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, username, [...activities], extras]); | |
| }, 500); |
🤖 Prompt for AI Agents
In apps/meteor/app/ui/client/lib/UserAction.ts around lines 41 to 44, the
broadcast uses shownName(getUser()) which can be undefined; compute a safe
username first (e.g., const username = shownName(getUser()) || '' or use a
fallback like 'Unknown') and use that variable in the sdk.publish call so the
emitted event never contains undefined for the username; update the payload to
use the safe username variable (or skip emitting if you prefer only when a real
username exists).
| emailOrUsername: typeof emailOrUsername === 'string' ? emailOrUsername : getUser()?.username, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure email 2FA always receives a string to avoid assertion errors and spurious recursion.
When neither argument nor server details provide email/username and getUser() is undefined, props become invalid. Default to empty string.
- emailOrUsername: typeof emailOrUsername === 'string' ? emailOrUsername : getUser()?.username,
+ emailOrUsername: typeof emailOrUsername === 'string' ? emailOrUsername : (getUser()?.username ?? ''),- emailOrUsername: emailOrUsername || error.details.emailOrUsername || getUser()?.username,
+ emailOrUsername: emailOrUsername ?? error.details.emailOrUsername ?? getUser()?.username ?? '',Also applies to: 111-114
🤖 Prompt for AI Agents
In apps/meteor/client/lib/2fa/process2faReturn.ts around lines 50-51 (and
similarly lines 111-114), the code passes emailOrUsername that can be undefined
when neither the argument nor getUser() yield a value, which breaks downstream
assertions and can trigger recursion; update the fallback logic so
emailOrUsername is always a string by using typeof checks and defaulting to an
empty string (e.g., if typeof emailOrUsername === 'string' ? emailOrUsername :
(getUser()?.username ?? '')), and apply the same pattern to the code at lines
111-114.
|
|
||
| export type ChatAPI = { | ||
| readonly uid: string | null; | ||
| readonly uid: string | undefined; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unify null/undefined semantics for userId.
You changed uid to string | undefined, but processSlashCommand still accepts userId: string | null. Recommend aligning to undefined to avoid dual semantics.
export type ChatAPI = {
- readonly uid: string | undefined;
+ readonly uid: string | undefined;
…
readonly flows: {
…
- readonly processSlashCommand: (message: IMessage, userId: string | null) => Promise<boolean>;
+ readonly processSlashCommand: (message: IMessage, userId: string | undefined) => Promise<boolean>;
…
};
};Run to spot remaining client usages of null userIds and any leftover Meteor.userId():
Also applies to: 153-153
🏁 Script executed:
#!/bin/bash
# Find client-side null-based userId types/usages
rg -nP --type=ts --type=tsx $'\buserId\s*:\s*string\s*\|\s*null\b' apps/meteor/client
# Ensure Meteor.userId() is no longer referenced in client code
rg -nP --type=ts --type=tsx $'\bMeteor\\.userId\\s*\\(' apps/meteor/clientLength of output: 55
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== userId: string | null matches =="
rg -nP '\buserId\s*:\s*string\s*\|\s*null\b' apps/meteor/client -S || true
echo "== Meteor.userId() matches =="
rg -nP '\bMeteor\.userId\s*\(' apps/meteor/client -S || true
echo "== general ': string | null' matches =="
rg -nP ':\s*string\s*\|\s*null' apps/meteor/client -S || true
echo "== processSlashCommand occurrences (context) =="
rg -nP 'processSlashCommand' apps/meteor/client -S -n -C3 || trueLength of output: 5416
Unify userId to undefined and align processSlashCommand signature
- Change remaining
userId: string | null→userId: string | undefined: apps/meteor/client/lib/VideoConfManager.ts:303, apps/meteor/client/lib/chats/ChatAPI.ts:153. - Align
processSlashCommanddeclaration with implementation/callers: implementation at apps/meteor/client/lib/chats/flows/processSlashCommand.ts isprocessSlashCommand(chat: ChatAPI, message: IMessage)and it's called from apps/meteor/client/lib/chats/flows/sendMessage.ts asprocessSlashCommand(chat, message)— update ChatAPI.flows.processSlashCommand or the implementation/call sites so the signature matches. - No client-side
Meteor.userId()usages found.
🤖 Prompt for AI Agents
In apps/meteor/client/lib/chats/ChatAPI.ts around lines 109 and 153, update the
types and function signature: change any remaining userId: string | null to
userId: string | undefined (so uid and other userId fields use undefined instead
of null), and modify the ChatAPI.flows.processSlashCommand declaration to match
its implementation and callers by accepting (chat: ChatAPI, message: IMessage) —
i.e., update the type/signature to (chat: ChatAPI, message: IMessage) =>
<existing return type> so callers like processSlashCommand(chat, message) and
the implementation in flows/processSlashCommand.ts are consistent.
| return await deriveKey(toArrayBuffer(getUserId()), baseKey); | ||
| } catch (error) { |
There was a problem hiding this comment.
Guard PBKDF2 salt against missing userId
toArrayBuffer(getUserId()) can be undefined, causing deriveKey to fail. Validate and emit a clearer error.
- return await deriveKey(toArrayBuffer(getUserId()), baseKey);
+ const uid = getUserId();
+ if (!uid) {
+ this.setState(E2EEState.ERROR);
+ return this.error('Missing userId for PBKDF2 salt');
+ }
+ return await deriveKey(toArrayBuffer(uid), baseKey);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return await deriveKey(toArrayBuffer(getUserId()), baseKey); | |
| } catch (error) { | |
| const uid = getUserId(); | |
| if (!uid) { | |
| this.setState(E2EEState.ERROR); | |
| return this.error('Missing userId for PBKDF2 salt'); | |
| } | |
| return await deriveKey(toArrayBuffer(uid), baseKey); | |
| } catch (error) { |
🤖 Prompt for AI Agents
In apps/meteor/client/lib/e2ee/rocketchat.e2e.ts around lines 556-557, the call
to deriveKey(toArrayBuffer(getUserId()), baseKey) can receive an undefined salt
because getUserId() may be undefined; before calling toArrayBuffer, validate
that getUserId() returns a non-empty value and if not, throw or reject with a
clear, descriptive error (e.g. "E2EE: missing userId required for PBKDF2 salt")
so deriveKey is never invoked with an invalid salt; update any surrounding error
handling to surface this message.
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
Outdated
Show resolved
Hide resolved
| userTemplate: ({ label }) => `<strong> ${label} </strong>`, | ||
| roomTemplate: ({ prefix, mention }) => `${prefix}<strong> ${mention} </strong>`, | ||
| }); |
There was a problem hiding this comment.
Escape mention labels to prevent HTML injection; remove stray spaces
Template values may contain unsafe chars. Also drop extra spaces inside .
- userTemplate: ({ label }) => `<strong> ${label} </strong>`,
- roomTemplate: ({ prefix, mention }) => `${prefix}<strong> ${mention} </strong>`,
+ userTemplate: ({ label }) => `<strong>${escapeHTML(label)}</strong>`,
+ roomTemplate: ({ prefix, mention }) => `${prefix}<strong>${escapeHTML(mention)}</strong>`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| userTemplate: ({ label }) => `<strong> ${label} </strong>`, | |
| roomTemplate: ({ prefix, mention }) => `${prefix}<strong> ${mention} </strong>`, | |
| }); | |
| userTemplate: ({ label }) => `<strong>${escapeHTML(label)}</strong>`, | |
| roomTemplate: ({ prefix, mention }) => `${prefix}<strong>${escapeHTML(mention)}</strong>`, | |
| }); |
tassoevan
force-pushed
the
chore/client-user-id
branch
from
86a6f14 to
83ca042
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/providers/UserProvider/UserProvider.tsx (1)
151-155: Emit logout with captured user to guarantee cleanup runs exactly once with the right payload.Capture the current user before calling
Meteor.logoutand emit it as the event payload in the logout callback.- logout: async () => Meteor.logout(), + logout: async () => { + const uid = userIdStore.getState(); + const prevUser = uid ? Users.state.get(uid) : undefined; + pendingLogoutUser = prevUser; + Meteor.logout(() => ee.emit('logout', prevUser)); + },
🧹 Nitpick comments (8)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
41-45: Tighten parsing of threadsListSize (explicit radix, avoid string coercion)Not related to this PR’s goal, but small safety/readability win.
Apply:
- useMemo(() => parseInt(`${getConfig('threadsListSize', 10)}`), []), + useMemo(() => Number.parseInt(String(getConfig('threadsListSize', 10)), 10), []),apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx (1)
27-27: Narrowuidbeforeincludesfor stronger TS safety/readability.Ternary narrows
uidtostringin the true branch and avoids potential strict-null-checks friction.- const following = !!uid && (thread.replies?.includes(uid) ?? false); + const following = uid ? (thread.replies?.includes(uid) ?? false) : false;apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (1)
43-49: Optional: support markdown in attachment title/description (pair with sanitizer above).If you adopt DOMPurify at injection, render attachment text via markdown for consistency.
- return escapeHTML(attachment.description); + return filterMarkdown(attachment.description);- return escapeHTML(attachment.title); + return filterMarkdown(attachment.title);apps/meteor/app/autotranslate/client/lib/autotranslate.ts (1)
83-87: Propagate autoTranslateShowInverse in recursive call.The recursive call drops the
autoTranslateShowInverseflag, changing behavior for nested attachments.Apply this diff:
- attachment.attachments = this.translateAttachments(attachment.attachments, language); + attachment.attachments = this.translateAttachments(attachment.attachments, language, autoTranslateShowInverse);apps/meteor/client/meteor/overrides/userAndUsers.ts (1)
9-12: Avoid redundant store writes in autorun.Guard
setStateto prevent emitting when value doesn’t change.Apply this diff:
- const userId = Accounts.connection.userId() ?? undefined; - userIdStore.setState(userId); + const userId = Accounts.connection.userId() ?? undefined; + if (userIdStore.getState() !== userId) { + userIdStore.setState(userId); + }apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
35-39: Consider aligning object overload to undefined‑only as well.To fully remove
nullfrom callers, mirror the change here by droppingnullfrom this overload.Apply this diff:
-export function getUserPreference<TValue>( - user: Pick<IUser, '_id' | 'settings'> | null | undefined, - key: string, - defaultValue: TValue, -): TValue; +export function getUserPreference<TValue>( + user: Pick<IUser, '_id' | 'settings'> | undefined, + key: string, + defaultValue: TValue, +): TValue;apps/meteor/client/providers/AuthorizationProvider.tsx (2)
27-28: Optional: move useUserId() before the suspend check to keep hook order stable across suspends.Not a blocker, but calling hooks in the same order across renders avoids edge cases when the component suspends.
- const isLoading = !PermissionsCachedStore.useReady(); + const userId = useUserId(); + const isLoading = !PermissionsCachedStore.useReady(); ... - const userId = useUserId();
29-39: queryRole: drop the optional marker on scope to avoid implicit-any optional param.The third param of hasRole is already optional; keeping “scope?” here may trigger implicit-any rules without adding value.
- queryRole: createReactiveSubscriptionFactory((role, scope?) => !!userId && hasRole(userId, role, scope)), + queryRole: createReactiveSubscriptionFactory((role, scope) => !!userId && hasRole(userId, role, scope)),Optional: since only queryRole depends on userId, you could hoist the other factories (queryPermission/AtLeastOne/All, getRoles, subscribeToRoles) into a memo with [] or top-level constants to keep their identities stable across userId changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (71)
apps/meteor/app/authorization/client/hasPermission.ts(3 hunks)apps/meteor/app/autotranslate/client/lib/autotranslate.ts(4 hunks)apps/meteor/app/lib/client/methods/sendMessage.ts(1 hunks)apps/meteor/app/otr/client/OTRRoom.ts(2 hunks)apps/meteor/app/reactions/client/methods/setReaction.ts(1 hunks)apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts(2 hunks)apps/meteor/app/ui/client/lib/ChatMessages.ts(2 hunks)apps/meteor/app/ui/client/lib/UserAction.ts(3 hunks)apps/meteor/app/utils/client/lib/getUserPreference.ts(2 hunks)apps/meteor/app/webrtc/client/WebRTCClass.ts(2 hunks)apps/meteor/client/apps/RealAppsEngineUIHost.ts(2 hunks)apps/meteor/client/cachedStores/PermissionsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/RoomsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/SubscriptionsCachedStore.ts(1 hunks)apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts(1 hunks)apps/meteor/client/hooks/useIdleConnection.ts(1 hunks)apps/meteor/client/lib/2fa/process2faReturn.ts(3 hunks)apps/meteor/client/lib/cachedStores/CachedStore.ts(2 hunks)apps/meteor/client/lib/cachedStores/index.ts(0 hunks)apps/meteor/client/lib/chats/ChatAPI.ts(1 hunks)apps/meteor/client/lib/chats/data.ts(6 hunks)apps/meteor/client/lib/chats/flows/processTooLongMessage.ts(2 hunks)apps/meteor/client/lib/chats/readStateManager.ts(2 hunks)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts(4 hunks)apps/meteor/client/lib/getPermaLink.ts(2 hunks)apps/meteor/client/lib/loggedIn.ts(1 hunks)apps/meteor/client/lib/mutationEffects/room.ts(1 hunks)apps/meteor/client/lib/mutationEffects/starredMessage.ts(1 hunks)apps/meteor/client/lib/normalizeThreadTitle.ts(0 hunks)apps/meteor/client/lib/user.ts(1 hunks)apps/meteor/client/lib/utils/getUidDirectMessage.ts(1 hunks)apps/meteor/client/lib/utils/timeAgo.ts(1 hunks)apps/meteor/client/meteor/minimongo/Cursor.ts(1 hunks)apps/meteor/client/meteor/minimongo/DiffSequence.ts(1 hunks)apps/meteor/client/meteor/minimongo/LocalCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/ObserveHandle.ts(1 hunks)apps/meteor/client/meteor/minimongo/common.ts(1 hunks)apps/meteor/client/meteor/overrides/ddpOverREST.ts(2 hunks)apps/meteor/client/meteor/overrides/unstoreLoginToken.ts(1 hunks)apps/meteor/client/meteor/overrides/userAndUsers.ts(1 hunks)apps/meteor/client/meteor/user.ts(1 hunks)apps/meteor/client/providers/AuthorizationProvider.tsx(2 hunks)apps/meteor/client/providers/SettingsProvider.tsx(1 hunks)apps/meteor/client/providers/UserProvider/UserProvider.tsx(4 hunks)apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts(1 hunks)apps/meteor/client/startup/accounts.ts(1 hunks)apps/meteor/client/startup/iframeCommands.ts(2 hunks)apps/meteor/client/startup/incomingMessages.ts(2 hunks)apps/meteor/client/startup/roles.ts(2 hunks)apps/meteor/client/startup/startup.ts(1 hunks)apps/meteor/client/stores/Messages.ts(1 hunks)apps/meteor/client/stores/Permissions.ts(1 hunks)apps/meteor/client/stores/Roles.ts(1 hunks)apps/meteor/client/stores/Rooms.ts(1 hunks)apps/meteor/client/stores/Settings.ts(1 hunks)apps/meteor/client/stores/Subscriptions.ts(1 hunks)apps/meteor/client/stores/Users.ts(1 hunks)apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts(1 hunks)apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts(1 hunks)apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts(1 hunks)apps/meteor/client/views/room/MemberListRouter.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts(1 hunks)apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts(1 hunks)apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts(1 hunks)apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/lib/cachedStores/index.ts
- apps/meteor/client/lib/normalizeThreadTitle.ts
🚧 Files skipped from review as they are similar to previous changes (58)
- apps/meteor/app/webrtc/client/WebRTCClass.ts
- apps/meteor/app/otr/client/OTRRoom.ts
- apps/meteor/client/lib/chats/ChatAPI.ts
- apps/meteor/client/lib/chats/flows/processTooLongMessage.ts
- apps/meteor/client/lib/2fa/process2faReturn.ts
- apps/meteor/client/lib/e2ee/rocketchat.e2e.ts
- apps/meteor/client/meteor/user.ts
- apps/meteor/app/lib/client/methods/sendMessage.ts
- apps/meteor/client/meteor/overrides/unstoreLoginToken.ts
- apps/meteor/client/lib/utils/timeAgo.ts
- apps/meteor/client/startup/startup.ts
- apps/meteor/client/lib/loggedIn.ts
- apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts
- apps/meteor/client/stores/Roles.ts
- apps/meteor/client/startup/accounts.ts
- apps/meteor/client/stores/Subscriptions.ts
- apps/meteor/client/lib/cachedStores/CachedStore.ts
- apps/meteor/client/meteor/minimongo/Cursor.ts
- apps/meteor/client/lib/mutationEffects/room.ts
- apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts
- apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts
- apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
- apps/meteor/client/stores/Settings.ts
- apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
- apps/meteor/app/ui/client/lib/ChatMessages.ts
- apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts
- apps/meteor/client/stores/Users.ts
- apps/meteor/client/apps/RealAppsEngineUIHost.ts
- apps/meteor/client/startup/iframeCommands.ts
- apps/meteor/client/startup/roles.ts
- apps/meteor/client/lib/chats/readStateManager.ts
- apps/meteor/client/lib/getPermaLink.ts
- apps/meteor/client/meteor/minimongo/LocalCollection.ts
- apps/meteor/app/ui/client/lib/UserAction.ts
- apps/meteor/client/meteor/minimongo/MinimongoCollection.ts
- apps/meteor/client/hooks/useIdleConnection.ts
- apps/meteor/client/lib/mutationEffects/starredMessage.ts
- apps/meteor/client/meteor/minimongo/common.ts
- apps/meteor/client/lib/user.ts
- apps/meteor/client/cachedStores/PermissionsCachedStore.ts
- apps/meteor/client/stores/Messages.ts
- apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
- apps/meteor/client/stores/Permissions.ts
- apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts
- apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts
- apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
- apps/meteor/client/stores/Rooms.ts
- apps/meteor/client/cachedStores/SubscriptionsCachedStore.ts
- apps/meteor/app/authorization/client/hasPermission.ts
- apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts
- apps/meteor/client/lib/chats/data.ts
- apps/meteor/client/providers/SettingsProvider.tsx
- apps/meteor/client/lib/utils/getUidDirectMessage.ts
- apps/meteor/app/reactions/client/methods/setReaction.ts
- apps/meteor/client/startup/incomingMessages.ts
- apps/meteor/client/meteor/minimongo/DiffSequence.ts
- apps/meteor/client/views/room/MemberListRouter.tsx
- apps/meteor/client/meteor/overrides/ddpOverREST.ts
🧰 Additional context used
🧬 Code graph analysis (8)
apps/meteor/app/autotranslate/client/lib/autotranslate.ts (2)
apps/meteor/client/meteor/user.ts (2)
watchUser(9-13)watchUserId(7-7)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx (1)
packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)
apps/meteor/client/providers/UserProvider/UserProvider.tsx (4)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)apps/meteor/client/stores/Users.ts (1)
Users(11-13)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts (1)
state(82-84)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (2)
packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(291-295)apps/meteor/app/emoji/client/emojiParser.ts (1)
emojiParser(7-75)
apps/meteor/client/meteor/overrides/userAndUsers.ts (3)
apps/meteor/tests/mocks/client/meteor.ts (2)
Tracker(44-44)Accounts(39-42)apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/meteor/user.ts (2)
watchUserId(7-7)watchUser(9-13)
apps/meteor/client/providers/AuthorizationProvider.tsx (2)
apps/meteor/client/lib/createReactiveSubscriptionFactory.ts (1)
createReactiveSubscriptionFactory(7-45)apps/meteor/app/authorization/client/hasRole.ts (1)
hasRole(6-21)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-253)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-253)
🪛 ast-grep (0.39.5)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
[warning] 55-55: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (18)
apps/meteor/client/meteor/minimongo/ObserveHandle.ts (1)
4-4: Approve: type import path change verified.queries.ts exports Query and all minimongo files import from './queries' (no remaining './Query' imports); type-only import — no runtime impact.
apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts (1)
4-4: No change required —pipeis a named export.apps/meteor/client/lib/cachedStores/pipe.ts declares
export function pipe(...)and existing imports use the named form, so keepimport { pipe } from '../../../../lib/cachedStores/pipe';.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
13-13: Ensure callers pass undefined (not null) and update strict null checksuseStreamUpdatesForMessageList already accepts uid: IUser['_id'] | undefined (apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts:50). rg returned no matches for null usages but reported "unrecognized file type: tsx" — TSX call sites may have been skipped. Re-run a repo-wide search including .tsx (or remove --type flags) for useThreadsList(...null),
uid: null, anduid === null; convert any null arguments to undefined and replace strict === null checks with nullish/undefined-safe checks (e.g., uid == null or uid === undefined).apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx (1)
21-21: Approve removal of?? undefined— types verifieduseUserId() returns useContext(UserContext).userId; UserContext declares userId: string | undefined (default undefined) and no assignments of
nullto userId were found in apps/packages.apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (3)
24-27: Don’t escape before markdown; compute mentions-free fast path correctly.Escaping first breaks markdown/mentions and can double‑escape. Also, truthiness checks on arrays mis-detect “no mentions/channels.”
Apply:
- const filteredMessage = filterMarkdown(escapeHTML(message.msg)); - if (!message.channels && !message.mentions) { - return filteredMessage; - } + const mdHtml = filterMarkdown(message.msg); + if ((message.channels?.length ?? 0) === 0 && (message.mentions?.length ?? 0) === 0) { + return mdHtml; + }
33-35: Escape mention template variables; remove stray spaces.Unescaped label/mention can inject HTML; extra spaces render oddly.
- userTemplate: ({ label }) => `<strong> ${label} </strong>`, - roomTemplate: ({ prefix, mention }) => `${prefix}<strong> ${mention} </strong>`, + userTemplate: ({ label }) => `<strong>${escapeHTML(label)}</strong>`, + roomTemplate: ({ prefix, mention }) => `${prefix}<strong>${escapeHTML(mention)}</strong>`,
55-56: Sanitize before dangerouslySetInnerHTML (XSS risk).The final HTML is injected without sanitization. Add a sanitizer (e.g., DOMPurify) right before injection.
Apply this diff:
- const innerHTML = useMemo(() => ({ __html: html }), [html]); - return <ContextualbarTitle dangerouslySetInnerHTML={innerHTML} />; + const sanitizedHtml = useMemo(() => DOMPurify.sanitize(html), [html]); + return <ContextualbarTitle dangerouslySetInnerHTML={{ __html: sanitizedHtml }} />;Add the import (outside the range):
+ import DOMPurify from 'dompurify';apps/meteor/app/autotranslate/client/lib/autotranslate.ts (2)
129-132: Non‑reactive getUserId() use is appropriate here.Event handler checks identity at call time and doesn’t need reactivity. Change looks good.
98-116: Reactive bridge confirmed — watchUserId and hasPermission are reactivewatch uses Tracker.currentComputation to subscribe/unsubscribe (apps/meteor/client/meteor/watch.ts); watchUserId uses it (apps/meteor/client/meteor/user.ts); hasPermission’s validator watches Users and Permissions (apps/meteor/app/authorization/client/hasPermission.ts), so granting/removing the "auto-translate" permission will invalidate the autorun. Note: PermissionsCachedStore.watchReady() can short‑circuit evaluation until the cache is ready.
apps/meteor/client/meteor/overrides/userAndUsers.ts (1)
14-21: Overrides look correct and align with demeteorization.
Meteor.userIdandMeteor.userdelegation to watch-helpers, and remappingMeteor.users, are consistent with the new store-based model.Please confirm test mocks cover these overrides (e.g.,
apps/meteor/tests/mocks/client/meteor.ts) and update if needed to avoid test flakiness.apps/meteor/app/utils/client/lib/getUserPreference.ts (2)
12-12: Type narrowing to undefined (no null) for userId overload — confirm callers don't pass null.
Matches the new getUserId() semantics. rg search forgetUserPreference(null)returned no matches; ensure call sites no longer passnull.
27-27: Verify no call sites pass null to getUserPreferenceDefault-value overload narrowing is good; my automated search failed (“No files were searched”), so I couldn't confirm absence of call sites passing null — run a repo-wide search for getUserPreference(...) and ensure no call uses null as the first argument.
apps/meteor/client/providers/UserProvider/UserProvider.tsx (4)
25-27: Import updates look good.Moving to
applyQueryOptionsdirect import and using the newuserIdStoreare consistent with the demeteorization goals.
68-69: Using the zustand store directly is fine here.Subscribing via
userIdStore()keeps the component reactive without pulling in Meteor reactivity.
117-118: Correct dependency for subscription source switching.Recomputing the factory when
userIdchanges prevents leaking subscriptions across auth boundaries.Ensure
Subscriptions.use/Rooms.useunsubscribe correctly when the factory flips. If not, we should add explicit disposers.
38-46: Logout cleanup can be skipped due to race (userId cleared before handler). Capture user and pass as payload.Current handler depends on
userIdStore.getState()which may beundefinedby the timeAccounts.onLogoutfires, skipping cleanup. Accept a payload and fall back only if absent; keep an ephemeral cache to bridge the window between initiating logout and the Accounts hook firing.Apply these diffs:
-import type { IRoom } from '@rocket.chat/core-typings'; +import type { IRoom, IUser } from '@rocket.chat/core-typings';const ee = new Emitter(); +let pendingLogoutUser: IUser | undefined; Accounts.onLogout(() => ee.emit('logout')); -ee.on('logout', async () => { - const userId = userIdStore.getState(); - if (!userId) return; - const user = Users.state.get(userId); - if (!user) return; +ee.on('logout', async (payloadUser?: IUser) => { + const user = + payloadUser ?? + pendingLogoutUser ?? + (() => { + const uid = userIdStore.getState(); + return uid ? Users.state.get(uid) : undefined; + })(); + pendingLogoutUser = undefined; + if (!user) return; await afterLogoutCleanUpCallback.run(user); await sdk.call('logoutCleanUp', user); });apps/meteor/client/providers/AuthorizationProvider.tsx (2)
3-3: Importing useMemo is appropriate for stabilizing the provider value.
1-1: Demeteorization import looks good — client Meteor.user/userId references found; confirm or remove
- apps/meteor/client/meteor/overrides/userAndUsers.ts — assigns Meteor.userId and Meteor.user.
- apps/meteor/client/views/room/hooks/useOpenRoom.ts:26 — comment referencing Meteor.user().
Confirm the overrides are intentional shims; convert any remaining runtime calls to useUserId/useUser.
| const user = watchUser(); | ||
| if (!user) return; | ||
|
|
||
| userLanguage = user.language || 'en'; | ||
| username = user.username || ''; | ||
| }); |
There was a problem hiding this comment.
Reset user-dependent globals on logout to avoid stale language/username.
When watchUser() becomes undefined (logout), userLanguage and username keep the last values, affecting translations until reload.
Apply this diff:
- const user = watchUser();
- if (!user) return;
-
- userLanguage = user.language || 'en';
- username = user.username || '';
+ const user = watchUser();
+ if (!user) {
+ userLanguage = 'en';
+ username = '';
+ return;
+ }
+ userLanguage = user.language || 'en';
+ username = user.username || '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const user = watchUser(); | |
| if (!user) return; | |
| userLanguage = user.language || 'en'; | |
| username = user.username || ''; | |
| }); | |
| const user = watchUser(); | |
| if (!user) { | |
| userLanguage = 'en'; | |
| username = ''; | |
| return; | |
| } | |
| userLanguage = user.language || 'en'; | |
| username = user.username || ''; | |
| }); |
🤖 Prompt for AI Agents
In apps/meteor/app/autotranslate/client/lib/autotranslate.ts around lines 23-28,
when watchUser() becomes undefined on logout the code currently returns early
and leaves userLanguage and username set to previous values; update the handler
so that if user is falsy (logout) you explicitly reset userLanguage = 'en' and
username = '' before returning, otherwise set them from
user.language/user.username as before.
| import type { SubscriptionWithRoom } from '@rocket.chat/ui-contexts'; | ||
|
|
||
| import { PrivateCachedStore } from '../lib/cachedStores'; | ||
| import { PrivateCachedStore } from '../lib/cachedStores/CachedStore'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Import path migration looks correct; verify consistent exports/usages repo-wide.
Directly importing from the concrete module reduces barrel coupling and potential cycles. Please confirm the symbol is a named export at the new path and that no files still import from the old barrel.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Confirm PrivateCachedStore is exported from CachedStore.ts"
rg -nP '\bexport\b[^{\n]*\bPrivateCachedStore\b|\bexport\s*{\s*[^}]*\bPrivateCachedStore\b' -g '**/CachedStore.ts'
echo "2) Find lingering barrel imports from ../lib/cachedStores"
rg -nP --type ts --type tsx -n -C1 "from ['\"]\.\.\/lib\/cachedStores['\"]"
echo "3) Detect default imports of PrivateCachedStore (should be named)"
rg -nP --type ts --type tsx -n "import\s+PrivateCachedStore\s+from"
echo "4) Confirm consistent direct imports to CachedStore"
rg -nP --type ts --type tsx -n -C1 "from ['\"][^\"]*lib\/cachedStores\/CachedStore['\"]"Length of output: 298
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Confirm PrivateCachedStore is exported (show surrounding lines)"
rg -nP -C2 "export\s+class\s+PrivateCachedStore\b" apps/meteor/client/lib/cachedStores/CachedStore.ts || true
echo
echo "2) Find imports that reference 'lib/cachedStores' but NOT '/CachedStore' (possible barrel/index imports)"
rg -nP -C1 "from\s+['\"][^'\"]*lib\/cachedStores(?!\/CachedStore)[^'\"]*['\"]" || true
echo
echo "3) Find direct imports from 'lib/cachedStores/CachedStore'"
rg -nP -C1 "from\s+['\"][^'\"]*lib\/cachedStores\/CachedStore['\"]" || true
echo
echo "4) Detect default imports of PrivateCachedStore"
rg -nP -n "import\s+PrivateCachedStore\s+from\s+['\"][^'\"]+['\"]" || true
echo
echo "5) List any occurrences of 'lib/cachedStores' for manual inspection"
rg -nP -n -C1 "lib\/cachedStores" || trueLength of output: 8527
PrivateCachedStore is a named export; update remaining barrel imports
- Verified: export class PrivateCachedStore exists at apps/meteor/client/lib/cachedStores/CachedStore.ts:382.
- No default-import usages detected.
- Action: replace barrel imports from '../cachedStores' with direct named imports from '../lib/cachedStores/CachedStore' where present — e.g. apps/meteor/client/providers/SettingsProvider.tsx:9 (import { PublicSettingsCachedStore, PrivateSettingsCachedStore } from '../cachedStores';).
🤖 Prompt for AI Agents
In apps/meteor/client/cachedStores/RoomsCachedStore.ts around line 5, the file
imports PrivateCachedStore via a barrel path; PrivateCachedStore is a named
export declared in apps/meteor/client/lib/cachedStores/CachedStore.ts so update
the import to import { PrivateCachedStore } from
'../lib/cachedStores/CachedStore'; and replace any other usages that import from
'../cachedStores' with direct named imports from
'../lib/cachedStores/CachedStore' (e.g., change imports in providers like
SettingsProvider.tsx) so they reference the concrete module where the class is
exported.
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
Show resolved
Hide resolved
tassoevan
force-pushed
the
chore/client-user-id
branch
from
83ca042 to
59388c5
Compare
tassoevan
force-pushed
the
chore/client-user-id
branch
from
59388c5 to
75653d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/meteor/client/lib/user.ts (1)
13-17: Consider adding an explicit return type annotation.The
getUserfunction relies on type inference. Adding an explicit return type annotation ((): IUser | undefined) would improve code clarity and make the API contract more explicit.Apply this diff to add the return type:
-export const getUser = () => { +export const getUser = (): IUser | undefined => { const userId = getUserId(); if (!userId) return undefined; return Users.state.get(userId); };apps/meteor/app/authorization/client/hasPermission.ts (1)
53-53: Optional: Simplify redundant nullish coalescing.The final
?? undefinedis redundant sincewatchUserId()already returnsIUser['_id'] | undefined.- userId = userId ?? watchUserId() ?? undefined; + userId = userId ?? watchUserId();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (77)
apps/meteor/app/authorization/client/hasPermission.ts(3 hunks)apps/meteor/app/autotranslate/client/lib/autotranslate.ts(4 hunks)apps/meteor/app/lib/client/methods/sendMessage.ts(1 hunks)apps/meteor/app/otr/client/OTRRoom.ts(2 hunks)apps/meteor/app/reactions/client/methods/setReaction.ts(1 hunks)apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts(2 hunks)apps/meteor/app/ui/client/lib/ChatMessages.ts(2 hunks)apps/meteor/app/ui/client/lib/UserAction.ts(3 hunks)apps/meteor/app/utils/client/lib/getUserPreference.ts(2 hunks)apps/meteor/app/webrtc/client/WebRTCClass.ts(2 hunks)apps/meteor/client/apps/RealAppsEngineUIHost.ts(2 hunks)apps/meteor/client/cachedStores/PermissionsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/RoomsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/SubscriptionsCachedStore.ts(1 hunks)apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts(1 hunks)apps/meteor/client/hooks/roomActions/useMediaCallRoomAction.ts(1 hunks)apps/meteor/client/hooks/useIdleConnection.ts(1 hunks)apps/meteor/client/lib/2fa/process2faReturn.ts(3 hunks)apps/meteor/client/lib/cachedStores/CachedStore.ts(2 hunks)apps/meteor/client/lib/cachedStores/index.ts(0 hunks)apps/meteor/client/lib/chats/ChatAPI.ts(1 hunks)apps/meteor/client/lib/chats/data.ts(6 hunks)apps/meteor/client/lib/chats/flows/processTooLongMessage.ts(2 hunks)apps/meteor/client/lib/chats/readStateManager.ts(2 hunks)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts(4 hunks)apps/meteor/client/lib/getPermaLink.ts(2 hunks)apps/meteor/client/lib/loggedIn.ts(1 hunks)apps/meteor/client/lib/mutationEffects/room.ts(1 hunks)apps/meteor/client/lib/mutationEffects/starredMessage.ts(1 hunks)apps/meteor/client/lib/normalizeThreadTitle.ts(0 hunks)apps/meteor/client/lib/user.ts(1 hunks)apps/meteor/client/lib/utils/getUidDirectMessage.ts(1 hunks)apps/meteor/client/lib/utils/timeAgo.ts(1 hunks)apps/meteor/client/meteor/minimongo/Cursor.ts(1 hunks)apps/meteor/client/meteor/minimongo/DiffSequence.ts(1 hunks)apps/meteor/client/meteor/minimongo/LocalCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/ObserveHandle.ts(1 hunks)apps/meteor/client/meteor/minimongo/common.ts(1 hunks)apps/meteor/client/meteor/overrides/ddpOverREST.ts(2 hunks)apps/meteor/client/meteor/overrides/unstoreLoginToken.ts(1 hunks)apps/meteor/client/meteor/overrides/userAndUsers.ts(1 hunks)apps/meteor/client/meteor/user.ts(1 hunks)apps/meteor/client/providers/AuthorizationProvider.tsx(2 hunks)apps/meteor/client/providers/SettingsProvider.tsx(1 hunks)apps/meteor/client/providers/UserProvider/UserProvider.tsx(4 hunks)apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts(1 hunks)apps/meteor/client/startup/accounts.ts(1 hunks)apps/meteor/client/startup/iframeCommands.ts(2 hunks)apps/meteor/client/startup/incomingMessages.ts(2 hunks)apps/meteor/client/startup/roles.ts(2 hunks)apps/meteor/client/startup/startup.ts(1 hunks)apps/meteor/client/stores/Messages.ts(1 hunks)apps/meteor/client/stores/Permissions.ts(1 hunks)apps/meteor/client/stores/Roles.ts(1 hunks)apps/meteor/client/stores/Rooms.ts(1 hunks)apps/meteor/client/stores/Settings.ts(1 hunks)apps/meteor/client/stores/Subscriptions.ts(1 hunks)apps/meteor/client/stores/Users.ts(1 hunks)apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts(1 hunks)apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts(1 hunks)apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts(1 hunks)apps/meteor/client/views/room/MemberListRouter.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts(1 hunks)apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts(1 hunks)apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts(1 hunks)apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx(1 hunks)apps/meteor/definition/externals/meteor/accounts-base.d.ts(1 hunks)packages/mock-providers/src/MockedAppRootBuilder.tsx(2 hunks)packages/ui-contexts/src/UserContext.ts(2 hunks)packages/ui-contexts/src/hooks/useUserId.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/lib/normalizeThreadTitle.ts
- apps/meteor/client/lib/cachedStores/index.ts
🚧 Files skipped from review as they are similar to previous changes (43)
- apps/meteor/client/cachedStores/PermissionsCachedStore.ts
- apps/meteor/client/cachedStores/SubscriptionsCachedStore.ts
- apps/meteor/client/startup/iframeCommands.ts
- apps/meteor/client/meteor/overrides/ddpOverREST.ts
- apps/meteor/client/lib/mutationEffects/starredMessage.ts
- apps/meteor/client/meteor/minimongo/ObserveHandle.ts
- apps/meteor/client/hooks/roomActions/useMediaCallRoomAction.ts
- apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts
- apps/meteor/app/ui/client/lib/ChatMessages.ts
- apps/meteor/app/webrtc/client/WebRTCClass.ts
- apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts
- apps/meteor/client/lib/chats/ChatAPI.ts
- apps/meteor/client/startup/roles.ts
- apps/meteor/client/cachedStores/RoomsCachedStore.ts
- apps/meteor/client/lib/getPermaLink.ts
- apps/meteor/app/autotranslate/client/lib/autotranslate.ts
- apps/meteor/client/meteor/user.ts
- apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
- apps/meteor/app/ui/client/lib/UserAction.ts
- apps/meteor/client/lib/chats/flows/processTooLongMessage.ts
- apps/meteor/client/lib/e2ee/rocketchat.e2e.ts
- packages/mock-providers/src/MockedAppRootBuilder.tsx
- apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
- apps/meteor/client/lib/2fa/process2faReturn.ts
- apps/meteor/client/lib/cachedStores/CachedStore.ts
- apps/meteor/client/startup/startup.ts
- apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
- apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts
- apps/meteor/client/stores/Subscriptions.ts
- apps/meteor/app/otr/client/OTRRoom.ts
- apps/meteor/client/meteor/minimongo/Cursor.ts
- apps/meteor/client/stores/Settings.ts
- apps/meteor/client/meteor/minimongo/LocalCollection.ts
- apps/meteor/app/lib/client/methods/sendMessage.ts
- apps/meteor/client/stores/Rooms.ts
- apps/meteor/client/lib/mutationEffects/room.ts
- packages/ui-contexts/src/hooks/useUserId.ts
- apps/meteor/client/stores/Users.ts
- apps/meteor/client/startup/incomingMessages.ts
- apps/meteor/client/views/room/MemberListRouter.tsx
- apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts
- apps/meteor/app/reactions/client/methods/setReaction.ts
- packages/ui-contexts/src/UserContext.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
PR: RocketChat/Rocket.Chat#37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/lib/utils/getUidDirectMessage.tsapps/meteor/client/providers/UserProvider/UserProvider.tsxapps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
PR: RocketChat/Rocket.Chat#37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/lib/utils/getUidDirectMessage.tsapps/meteor/client/providers/UserProvider/UserProvider.tsxapps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts
🧬 Code graph analysis (21)
apps/meteor/client/hooks/useIdleConnection.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/lib/utils/getUidDirectMessage.ts (3)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/IUser.ts (1)
IUser(186-255)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (2)
apps/meteor/app/livechat/server/methods/getFirstRoomMessage.ts (1)
uid(17-35)packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/lib/chats/data.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts (1)
packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)
apps/meteor/client/providers/AuthorizationProvider.tsx (3)
apps/meteor/client/lib/createReactiveSubscriptionFactory.ts (1)
createReactiveSubscriptionFactory(7-45)apps/meteor/app/authorization/client/hasPermission.ts (3)
hasPermission(81-81)hasAtLeastOnePermission(72-73)hasAllPermission(66-70)apps/meteor/app/authorization/client/hasRole.ts (1)
hasRole(6-21)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx (1)
packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)
apps/meteor/client/providers/UserProvider/UserProvider.tsx (4)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)apps/meteor/client/stores/Users.ts (1)
Users(11-13)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts (1)
state(82-84)
apps/meteor/client/apps/RealAppsEngineUIHost.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/meteor/overrides/userAndUsers.ts (3)
apps/meteor/tests/mocks/client/meteor.ts (2)
Tracker(44-44)Accounts(39-42)apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/meteor/user.ts (2)
watchUserId(7-7)watchUser(9-13)
apps/meteor/client/lib/user.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/lib/chats/readStateManager.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/startup/accounts.ts (3)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/cachedStores/index.ts (2)
SubscriptionsCachedStore(5-5)PublicSettingsCachedStore(3-3)apps/meteor/client/lib/userData.ts (1)
useUserDataSyncReady(8-8)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (2)
packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(306-310)apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (1)
useNormalizedThreadTitleHtml(10-49)
apps/meteor/client/lib/utils/timeAgo.ts (2)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
getUserPreference(40-47)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/loggedIn.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (3)
packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(306-310)packages/ui-contexts/src/index.ts (2)
useUser(80-80)useSetting(69-69)apps/meteor/app/emoji/client/emojiParser.ts (1)
emojiParser(7-75)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/app/authorization/client/hasPermission.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts (2)
apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
getUserPreference(40-47)apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
🪛 ast-grep (0.39.5)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
[warning] 14-14: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (48)
apps/meteor/client/providers/SettingsProvider.tsx (1)
10-10: LGTM! Clean import path refactor.The change from barrel export to direct module import improves tree-shaking and aligns with the repository's modularization efforts.
apps/meteor/client/stores/Messages.ts (1)
3-4: LGTM! Import paths verified.The refactoring from barrel imports to targeted imports is correct. Both
createDocumentMapStoreandcreateGlobalStoreare properly exported from their respective modules, and the usage remains unchanged.apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts (1)
4-4: Verified: Import path update is correct and consistent.The verification confirms:
pipeis properly exported fromapps/meteor/client/lib/cachedStores/pipe.ts- All three files importing
pipeuse the new path consistently- No old import patterns remain in the codebase
The import path change is part of a complete migration and poses no issues.
apps/meteor/definition/externals/meteor/accounts-base.d.ts (1)
80-82: LGTM! Type declaration correctly typed and actively used.The
connectionproperty with itsuserId()method is used inapps/meteor/client/meteor/overrides/userAndUsers.tsas part of the migration fromMeteor.userId()to watcher/store-based user resolution. The return typestring | nullis correct.apps/meteor/client/meteor/minimongo/MinimongoCollection.ts (1)
4-5: Import path refactoring verified successfully.The verification confirms that:
- Old paths (
./Queryand./DocumentMapStore) have been properly removed- New paths (
./queriesand../../lib/cachedStores/DocumentMapStore) exist and correctly export the required symbolsThe refactoring is complete and consistent with the project-wide consolidation effort.
apps/meteor/client/meteor/minimongo/common.ts (1)
3-3: Import path updated consistently.The import path for
entriesOfhas been updated to match the same pattern as inDiffSequence.ts. This change is consistent with the file reorganization.apps/meteor/client/meteor/minimongo/DiffSequence.ts (1)
4-4: Import path update verified successfully.The import path change from
../objectUtilsto../../lib/objectUtilsis correct. The utility file exists atapps/meteor/client/lib/objectUtils.tsand properly exports theentriesOffunction. All files in theminimongodirectory have been consistently updated to use the new path.apps/meteor/client/meteor/overrides/unstoreLoginToken.ts (1)
3-3: Import path update verified and complete.All imports of
CachedStoresManageracross the codebase have been updated to use direct module imports. Only two files import this module:
apps/meteor/client/lib/cachedStores/CachedStore.tsuses'./CachedStoresManager'apps/meteor/client/meteor/overrides/unstoreLoginToken.tsuses'../../lib/cachedStores/CachedStoresManager'No barrel export file exists in the
cachedStoresdirectory, and no references to the old import path remain. The change is consistent with the PR's refactoring objectives.apps/meteor/client/hooks/useIdleConnection.ts (2)
1-1: LGTM: Type import aligns with type safety improvements.The import of
IUserfrom@rocket.chat/core-typingssupports the parameter type refinement on line 7, consistent with the PR's goal of standardizing user ID handling across the codebase.
7-7: Type change verified and consistent with caller.The parameter type change from
string | nulltoIUser['_id'] | undefinedis correct. Verification confirms thatuserIdStore(the source ofuserIdpassed to this hook) already returnsIUser['_id'] | undefined, ensuring type consistency across the codebase.apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx (1)
21-21: LGTM: Redundant nullish coalescing removed.The
?? undefinedwas unnecessary sinceuseUserId()already returnsstring | undefined. The removal simplifies the code without changing behavior.apps/meteor/client/stores/Permissions.ts (1)
3-4: LGTM: Import refactoring aligns with PR pattern.Splitting the barrel import into explicit module imports improves tree-shaking and follows the project-wide refactoring pattern seen across multiple store files.
apps/meteor/client/stores/Roles.ts (1)
3-4: LGTM: Consistent import refactoring.Same pattern as Permissions.ts—splitting barrel imports into explicit imports without changing functionality.
apps/meteor/client/lib/utils/getUidDirectMessage.ts (2)
4-4: LGTM: Replaces Meteor.userId() with getUserId().This change aligns with the PR's demeteorization objective by introducing the centralized
getUserId()helper.
6-6: No issues found—the single caller is compatible with the type change.The only call site at
apps/meteor/app/otr/client/OTRRoom.ts:65invokesgetUidDirectMessage(rid)without passing a second argument, relying on the default parameter. Since no caller explicitly passesnull, the change fromIUser['_id'] | nulltoIUser['_id'] | undefinedis safe and non-breaking.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
13-13: No issues found—types are already aligned.The verification confirms:
- useUserId returns
string | undefined, not null- The only call site in ThreadList.tsx line 110 passes
uidfrom useUserId()- Types are compatible:
string | undefinedmatchesIUser['_id'] | undefinedThe type change from
nulltoundefinedis consistent with the existing codebase conventions.apps/meteor/client/lib/user.ts (2)
6-9: Verify the private consumption guidance.The comment states "do not consume this store directly -- consume it via UserContext," but multiple files in this PR directly import
getUserIdandgetUserfrom this module (e.g.,readStateManager.ts,timeAgo.ts,loggedIn.ts). This suggests either the comment is outdated or there's a misalignment between the intended usage pattern and actual usage. Please clarify whether direct consumption ofgetUserId/getUseris acceptable or if all consumers should go throughUserContext.
11-11: LGTM!The
getUserIdfunction correctly implements a non-reactive getter for the current user ID from the store.apps/meteor/client/lib/chats/readStateManager.ts (1)
77-81: Verify behavior when user is not logged in.The comparison
record.u._id !== getUserId()on line 79 will evaluate totruefor all messages whengetUserId()returnsundefined(user not logged in). While the subscription check on line 73 suggests a user should be logged in at this point, the logic doesn't explicitly guard against this edge case. Please verify thatupdateFirstUnreadRecordIdis only called when a user is logged in, or add an explicit check.If an explicit check is needed, consider this diff:
private updateFirstUnreadRecordId() { - if (!this.subscription?.ls) { + const userId = getUserId(); + if (!this.subscription?.ls || !userId) { return; } const firstUnreadRecord = Messages.state.findFirst( (record) => - record.rid === this.subscription?.rid && record.ts.getTime() > this.subscription.ls.getTime() && record.u._id !== getUserId(), + record.rid === this.subscription?.rid && record.ts.getTime() > this.subscription.ls.getTime() && record.u._id !== userId, (a, b) => a.ts.getTime() - b.ts.getTime(), ); this.setFirstUnreadRecordId(firstUnreadRecord?._id); }apps/meteor/client/lib/utils/timeAgo.ts (1)
13-13: LGTM!The replacement of
Meteor.userId()withgetUserId()is correct, and the types are compatible withgetUserPreference.apps/meteor/client/lib/loggedIn.ts (1)
5-8: LGTM!The migration from
Meteor.userId()togetUserId()is correct, and the boolean coercion properly handles both string and undefined values.apps/meteor/client/providers/AuthorizationProvider.tsx (1)
1-3: LGTM! Proper migration to non-reactive user ID with correct memoization.The migration from
Meteor.userId()touseUserId()is correctly implemented. TheuseMemohook with[userId]dependency ensurescontextValueis only recreated when the user ID changes, preventing unnecessary re-renders of context consumers. The!!userIdguard on line 34 safely handlesundefinedbefore callinghasRole.Also applies to: 27-39
apps/meteor/client/lib/chats/data.ts (1)
18-18: LGTM! Consistent migration from Meteor.userId() to getUserId().All five replacements correctly handle the return type change from
string | nulltostring | undefined. The explicitundefinedchecks at lines 100-104 and 183-187 properly guard against missing user IDs, and the equality comparisons at lines 82 and 205 remain safe.Also applies to: 82-82, 100-100, 138-138, 183-183, 205-205
apps/meteor/client/meteor/overrides/userAndUsers.ts (4)
1-2: LGTM! Correct imports for the new user identity infrastructure.The imports properly bring in
Accounts,Meteor, the newuserIdStore, and the reactive watch helperswatchUserandwatchUserId.Also applies to: 5-7
9-12: LGTM! Correct bridge from Meteor's reactive user ID to Zustand store.The
Tracker.autoruncorrectly synchronizesAccounts.connection.userId()changes intouserIdStore, with the?? undefinedcoercion ensuring the store's type contract (IUser['_id'] | undefined) is maintained.
19-20: LGTM! Type assertion is correctly documented.The comment clearly explains why the type assertion is needed:
IUserhas more required fields thanMeteor.User. This is a valid compatibility bridge for the legacy Meteor API.
14-14: Reactive behavior verified—watch()correctly handles both contexts.The
watch()utility implementation confirms proper handling:
- Inside reactive context (
Tracker.currentComputationexists): subscribes to store changes and invalidates computation when values change- Outside reactive context: returns current value via
store.getState()without errorThe
?? nullcoercion preserves Meteor's API contract as intended.apps/meteor/app/authorization/client/hasPermission.ts (3)
5-5: LGTM: Import aligns with demeteorization initiative.The addition of
watchUserIdimport replaces the previous Meteor-based user identity sourcing.
59-61: LGTM: Readiness check prevents premature permission evaluation.The guard ensures permissions are only evaluated after the store is populated, preventing false negatives during initialization.
50-50: Type narrowing verified—no callers found in codebase.Searched the entire codebase and found zero call sites for
userHasAllPermission. The function is exported from the authorization client module but remains unused internally. The type change fromuserId?: IUser['_id'] | nulltouserId?: IUser['_id']poses no risk to internal code and aligns with the broader pattern of removingnullfrom union types across the codebase.apps/meteor/app/utils/client/lib/getUserPreference.ts (2)
12-12: LGTM: Type narrowing aligns with userId standardization.The removal of
nullfrom the string userId overloads is consistent with the broader migration to useundefinedfor absent userId values. The user object overloads (lines 19, 35-39) intentionally retainnullin their union.Also applies to: 27-27
40-47: LGTM: Implementation correctly handles all overloads.The unified implementation properly discriminates between string userId and user objects, with appropriate fallback to global settings.
apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts (2)
10-10: LGTM: Import of non-reactive userId getter.The addition of
getUserIdappropriately replaces Meteor-based user identity sourcing with the centralized non-reactive getter.
142-142: LGTM: Correct use of non-reactive userId getter.The replacement of
Meteor.userId()withgetUserId()is appropriate. The non-reactive getter is the right choice for this context (called during async message loading), and the types align with the updatedgetUserPreferencesignature.apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts (1)
1-1: LGTM! Type annotation improves clarity.The addition of the type-only import and explicit parameter type annotation aligns with the PR's goal of standardizing user ID types to
undefinedinstead ofnull.Also applies to: 7-7
apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts (1)
3-3: LGTM! Import path updated consistently.The import path change from
../lib/cachedStoresto../lib/cachedStores/CachedStorealigns with the broader refactoring across other cached store files in this PR.apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (1)
5-5: Hook-based approach addresses maintainability concern.The refactor to use
useNormalizedThreadTitleHtmladdresses the maintainability concern raised in previous reviews about removingnormalizeThreadTitle. The hook provides better isolation and testability compared to a utility function.Note: The XSS sanitization concerns from past reviews now apply to the hook implementation (reviewed separately).
Also applies to: 12-14
apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (3)
19-19: Critical: Escape after markdown filtering, not before.Escaping HTML before filtering markdown will prevent markdown from being processed correctly and may cause double-escaping. The markdown filter should operate on raw text, and escaping should happen on the final output.
Apply this diff:
- const filteredMessage = filterMarkdown(escapeHTML(message.msg)); + const filteredMessage = escapeHTML(filterMarkdown(message.msg));
28-29: Critical: Escape template values to prevent XSS; remove extra spaces.The template functions inject
labelandmentionvalues directly into HTML without escaping, creating an XSS vulnerability. Additionally, the extra spaces inside the<strong>tags are unnecessary.Apply this diff:
- userTemplate: ({ label }) => `<strong> ${label} </strong>`, - roomTemplate: ({ prefix, mention }) => `${prefix}<strong> ${mention} </strong>`, + userTemplate: ({ label }) => `<strong>${escapeHTML(label)}</strong>`, + roomTemplate: ({ prefix, mention }) => `${escapeHTML(prefix)}<strong>${escapeHTML(mention)}</strong>`,
31-32: Critical: Pass raw text to MentionsParser and fix parsing order.The current implementation:
- Escapes and filters the message into
filteredMessage(HTML string)- Passes this HTML to
emojiParser(line 31)- Passes the escaped text as
msgtoinstance.parse(line 32)This breaks mention parsing because
MentionsParserexpects raw text in themsgfield to identify mentions correctly. The escaped HTML will not match mention patterns.Apply this diff (after fixing line 19):
- const instance = new MentionsParser({ - pattern: () => pattern, - useRealName: () => useRealName, - me: () => me, - userTemplate: ({ label }) => `<strong>${escapeHTML(label)}</strong>`, - roomTemplate: ({ prefix, mention }) => `${escapeHTML(prefix)}<strong>${escapeHTML(mention)}</strong>`, - }); - const html = emojiParser(filteredMessage); - return instance.parse({ ...message, msg: filteredMessage, html }).html ?? ''; + const instance = new MentionsParser({ + pattern: () => pattern, + useRealName: () => useRealName, + me: () => me, + userTemplate: ({ label }) => `<strong>${escapeHTML(label)}</strong>`, + roomTemplate: ({ prefix, mention }) => `${escapeHTML(prefix)}<strong>${escapeHTML(mention)}</strong>`, + }); + const parsedHtml = instance.parse({ ...message, msg: message.msg, html: filteredMessage }).html ?? ''; + return emojiParser(parsedHtml);This ensures:
- MentionsParser receives raw text in
msgfield- Filtered markdown HTML is passed as the
htmlfield for template rendering- Emoji parsing happens last, after mentions are resolved
apps/meteor/client/startup/accounts.ts (2)
25-39: Critical: Temporal Dead Zone (TDZ) bug if subscriptions fire synchronously.If any of the
subscribecalls invokecheckAndResolvesynchronously (which can happen if the store already has the target state), the function will attempt to callunsubscribeUserId(),unsubscribeSubscriptions(), etc., before those variables are assigned, causing aReferenceError.Apply this diff to predeclare the unsubscribe functions:
return new Promise((resolve) => { + let unsubscribeUserId: () => void = () => {}; + let unsubscribeSubscriptions: () => void = () => {}; + let unsubscribeSettings: () => void = () => {}; + let unsubscribeUserData: () => void = () => {}; + const checkAndResolve = () => { if (!isMainReady()) return; unsubscribeUserId(); unsubscribeSubscriptions(); unsubscribeSettings(); unsubscribeUserData(); resolve(); }; - const unsubscribeUserId = userIdStore.subscribe(checkAndResolve); - const unsubscribeSubscriptions = SubscriptionsCachedStore.useReady.subscribe(checkAndResolve); - const unsubscribeSettings = PublicSettingsCachedStore.useReady.subscribe(checkAndResolve); - const unsubscribeUserData = useUserDataSyncReady.subscribe(checkAndResolve); + unsubscribeUserId = userIdStore.subscribe(checkAndResolve); + unsubscribeSubscriptions = SubscriptionsCachedStore.useReady.subscribe(checkAndResolve); + unsubscribeSettings = PublicSettingsCachedStore.useReady.subscribe(checkAndResolve); + unsubscribeUserData = useUserDataSyncReady.subscribe(checkAndResolve); });
11-23: Verified: Tracker removal and async error handling are correct.The migration from
Tracker.autorunto Promise-based readiness checking is sound:
userId undefined handling:
userIdStoreinitializes toundefined(line 9 ofclient/lib/user.ts), which correctly represents "no user logged in." The logicif (!uid) return trueproperly treats this as an immediate ready state.Error propagation: The async callback in
Accounts.verifyEmail(lines 46-48) correctly handles errors by displaying a toast message and throwing aMeteor.Error, ensuring failures are not silently swallowed.The implementation matches the existing
useMainReadyhook pattern and properly subscribes to all required stores.apps/meteor/client/providers/UserProvider/UserProvider.tsx (6)
1-1: LGTM: Import changes align with store-based architecture.The migration from Meteor globals to centralized stores is correctly reflected in the imports. The
userIdStoreprovides non-reactive userId access, and the newapplyQueryOptionspath suggests proper modularization of cached store utilities.Also applies to: 25-25, 27-27
68-68: LGTM: Correct reactive store access pattern.
userIdStore()correctly replacesMeteor.userId()with reactive Zustand store access, ensuring the component re-renders when userId changes.
70-74: LGTM: Efficient user data retrieval with proper null handling.The selector pattern correctly retrieves user data from the Users store with appropriate null guards. This ensures the component only re-renders when the specific user entity changes, not on every store update.
92-118: LGTM: Conditional subscription factory correctly adapts to auth state.The factory correctly switches between
Subscriptions(authenticated) andRooms(guest) stores based onuserIdpresence. The manual subscription pattern with snapshot comparison efficiently prevents unnecessary re-renders.The acknowledged type assertion issue (line 112) is acceptable as a temporary measure given the noted future refactoring plans.
141-157: LGTM: Context value correctly exposes store-based user state.The context properly integrates the new
userIdStore-based approach while maintaining the existing API surface. The delegation toMeteor.logout()at line 151 is appropriate as this PR targets userId retrieval, not the authentication mechanism itself.
38-46: Potential double execution of logout cleanup persists.The logout handler still duplicates cleanup logic present in
apps/meteor/client/startup/iframeCommands.ts(lines 79-88). Both paths invokeafterLogoutCleanUpCallback.run(user)andsdk.call('logoutCleanUp', user), risking duplicate execution if both trigger.Consider consolidating cleanup to a single trigger point or implementing idempotency guards.
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
Show resolved
Hide resolved
tassoevan
force-pushed
the
chore/client-user-id
branch
from
9d5b7aa to
6571ff6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts (1)
41-41: Potential edge case: undefined === undefined allows unintended deletion.When both
file.userIdanduidareundefined, the strict equality check returnstrue, which would incorrectly classify a file with no owner as belonging to a non-authenticated user. This could allow deletion when neither the file nor the user has an associated ID.Consider adding an explicit guard to ensure both values are defined before comparing:
-const isUserOwnFile = (file: IUpload) => file.userId === uid; +const isUserOwnFile = (file: IUpload) => uid !== undefined && file.userId === uid;This ensures that anonymous users (where
uidisundefined) cannot delete files with no owner (whereuserIdisundefined).apps/meteor/app/utils/client/lib/getUserPreference.ts (1)
19-19: Ensure consistent nullability in getUserPreference overloads
The object-based overload inapps/meteor/app/utils/client/lib/getUserPreference.ts(line 19) acceptsnull | undefinedfor the user argument, while the string-based overload only allowsundefined. Since no existing call sites passnull, change the object overload toPick<IUser, '_id' | 'settings'> | undefinedto align both overloads.
♻️ Duplicate comments (3)
apps/meteor/app/ui/client/lib/UserAction.ts (1)
41-44: Guard against undefined username in activity broadcast
shownName(getUser())can beundefined(e.g., before user record loads or when user is not logged in). Avoid emitting an event with an undefined username.Apply this diff to add a guard:
-const emitActivities = debounce(async (rid: string, extras: IExtras): Promise<void> => { - const activities = roomActivities.get(extras?.tmid || rid) || new Set(); - sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, shownName(getUser()), [...activities], extras]); -}, 500); +const emitActivities = debounce(async (rid: string, extras: IExtras): Promise<void> => { + const activities = roomActivities.get(extras?.tmid || rid) || new Set(); + const username = shownName(getUser()); + if (!username) return; + sdk.publish('notify-room', [`${rid}/${USER_ACTIVITY}`, username, [...activities], extras]); +}, 500);apps/meteor/client/startup/accounts.ts (1)
25-38: Predeclare unsubscribe placeholders to avoid TDZ crash.Line 35 subscribes before assigning, so if any store fires the listener synchronously we hit a ReferenceError because
unsubscribeUserId, etc. are still in the TDZ. Please initialize them as no-op functions beforecheckAndResolveand then reassign after each subscribe call.- return new Promise((resolve) => { - const checkAndResolve = () => { + return new Promise((resolve) => { + let unsubscribeUserId: () => void = () => {}; + let unsubscribeSubscriptions: () => void = () => {}; + let unsubscribeSettings: () => void = () => {}; + let unsubscribeUserData: () => void = () => {}; + + const checkAndResolve = () => { if (!isMainReady()) return; unsubscribeUserId(); unsubscribeSubscriptions(); unsubscribeSettings(); unsubscribeUserData(); resolve(); }; - const unsubscribeUserId = userIdStore.subscribe(checkAndResolve); - const unsubscribeSubscriptions = SubscriptionsCachedStore.useReady.subscribe(checkAndResolve); - const unsubscribeSettings = PublicSettingsCachedStore.useReady.subscribe(checkAndResolve); - const unsubscribeUserData = useUserDataSyncReady.subscribe(checkAndResolve); + unsubscribeUserId = userIdStore.subscribe(checkAndResolve); + unsubscribeSubscriptions = SubscriptionsCachedStore.useReady.subscribe(checkAndResolve); + unsubscribeSettings = PublicSettingsCachedStore.useReady.subscribe(checkAndResolve); + unsubscribeUserData = useUserDataSyncReady.subscribe(checkAndResolve); });apps/meteor/client/providers/UserProvider/UserProvider.tsx (1)
38-46: Duplicate cleanup concern already flagged.This logout handler executes
afterLogoutCleanUpCallback.runandsdk.call('logoutCleanUp'), which duplicates the same cleanup inapps/meteor/client/startup/iframeCommands.ts(lines 79-88). A previous review comment already identified this issue and requested verification/deduplication.
🧹 Nitpick comments (7)
apps/meteor/app/otr/client/OTRRoom.ts (1)
106-108: Consider simplifying the user existence check.The
uservariable is retrieved but never used after the early-return guard. If the intent is purely to verify an authenticated session exists before posting the OTR refresh, consider usinggetUserId()directly for clarity:-const user = getUser(); -if (!user) return; +if (!getUserId()) return;Alternatively, if future logic requires the user object, keep the current implementation and add a comment explaining the defensive check.
apps/meteor/client/meteor/overrides/userAndUsers.ts (1)
9-12: Consider using public APIAccounts.userId()instead ofAccounts.connection.userId().
Accounts.connectionis an internal Meteor API that's not officially documented. The publicAccounts.userId()API provides the same functionality and is reactive withinTracker.autorun, making it a more stable choice.Apply this diff to use the public API:
Tracker.autorun(() => { - const userId = Accounts.connection.userId() ?? undefined; + const userId = Accounts.userId() ?? undefined; userIdStore.setState(userId); });apps/meteor/client/meteor/overrides/ddpOverREST.ts (1)
72-72: Consider updating comment to reflect new user ID architecture.The comment references "Meteor.userId is set", which may be outdated given the migration to
getUserId()/userIdStore. While the underlying Meteor mechanism may still setMeteor.userIdinternally, consider updating the comment for consistency with the new architecture.apps/meteor/client/lib/utils/getUidDirectMessage.ts (1)
6-6: Simplify redundant nullish coalescing.The default parameter
uid = getUserId() ?? undefinedis redundant sincegetUserId()already returnsIUser['_id'] | undefined.-export const getUidDirectMessage = (rid: IRoom['_id'], uid: IUser['_id'] | undefined = getUserId() ?? undefined): string | undefined => { +export const getUidDirectMessage = (rid: IRoom['_id'], uid: IUser['_id'] | undefined = getUserId()): string | undefined => {apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (1)
48-48: Consider more granular memoization dependencies.The
useMemodepends onmainMessageobject reference, which may cause unnecessary re-computations if the parent component creates a new object on each render (even with identical content).If the parent doesn't guarantee stable
mainMessagereferences, consider depending on specific fields:- }, [mainMessage, me, pattern, useRealName]); + }, [mainMessage._id, mainMessage.msg, mainMessage.attachments, mainMessage.mentions, mainMessage.channels, me, pattern, useRealName]);However, if
mainMessageis stable (e.g., from a database query hook with object identity), the current approach is fine.apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts (1)
50-50: Approve uid change; consider unifying null/undefined for rid
The switch fromnulltoundefinedfor theuidparameter correctly reflectsgetUserId()’s return type, and all internal call sites now propagateuidwithout passing a literalnull. The existing guard (if (!uid || !rid)) continues to work.Optional: for consistency, you may later migrate the
ridparameter fromIRoom['_id'] | nulltoIRoom['_id'] | undefinedor document the mixed null/undefined pattern.apps/meteor/app/authorization/client/hasPermission.ts (1)
53-53: Consider removing redundant?? undefined.Since
watchUserId()already returnsIUser['_id'] | undefined, the final?? undefinedis redundant and can be simplified:- userId = userId ?? watchUserId() ?? undefined; + userId = userId ?? watchUserId();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (77)
apps/meteor/app/authorization/client/hasPermission.ts(3 hunks)apps/meteor/app/autotranslate/client/lib/autotranslate.ts(4 hunks)apps/meteor/app/lib/client/methods/sendMessage.ts(1 hunks)apps/meteor/app/otr/client/OTRRoom.ts(2 hunks)apps/meteor/app/reactions/client/methods/setReaction.ts(1 hunks)apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts(2 hunks)apps/meteor/app/ui/client/lib/ChatMessages.ts(2 hunks)apps/meteor/app/ui/client/lib/UserAction.ts(3 hunks)apps/meteor/app/utils/client/lib/getUserPreference.ts(2 hunks)apps/meteor/app/webrtc/client/WebRTCClass.ts(2 hunks)apps/meteor/client/apps/RealAppsEngineUIHost.ts(2 hunks)apps/meteor/client/cachedStores/PermissionsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/RoomsCachedStore.ts(1 hunks)apps/meteor/client/cachedStores/SubscriptionsCachedStore.ts(1 hunks)apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts(1 hunks)apps/meteor/client/hooks/roomActions/useMediaCallRoomAction.ts(1 hunks)apps/meteor/client/hooks/useIdleConnection.ts(1 hunks)apps/meteor/client/lib/2fa/process2faReturn.ts(3 hunks)apps/meteor/client/lib/cachedStores/CachedStore.ts(2 hunks)apps/meteor/client/lib/cachedStores/index.ts(0 hunks)apps/meteor/client/lib/chats/ChatAPI.ts(1 hunks)apps/meteor/client/lib/chats/data.ts(6 hunks)apps/meteor/client/lib/chats/flows/processTooLongMessage.ts(2 hunks)apps/meteor/client/lib/chats/readStateManager.ts(2 hunks)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts(4 hunks)apps/meteor/client/lib/getPermaLink.ts(2 hunks)apps/meteor/client/lib/loggedIn.ts(1 hunks)apps/meteor/client/lib/mutationEffects/room.ts(1 hunks)apps/meteor/client/lib/mutationEffects/starredMessage.ts(1 hunks)apps/meteor/client/lib/normalizeThreadTitle.ts(0 hunks)apps/meteor/client/lib/user.ts(1 hunks)apps/meteor/client/lib/utils/getUidDirectMessage.ts(1 hunks)apps/meteor/client/lib/utils/timeAgo.ts(1 hunks)apps/meteor/client/meteor/minimongo/Cursor.ts(1 hunks)apps/meteor/client/meteor/minimongo/DiffSequence.ts(1 hunks)apps/meteor/client/meteor/minimongo/LocalCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts(1 hunks)apps/meteor/client/meteor/minimongo/ObserveHandle.ts(1 hunks)apps/meteor/client/meteor/minimongo/common.ts(1 hunks)apps/meteor/client/meteor/overrides/ddpOverREST.ts(2 hunks)apps/meteor/client/meteor/overrides/unstoreLoginToken.ts(1 hunks)apps/meteor/client/meteor/overrides/userAndUsers.ts(1 hunks)apps/meteor/client/meteor/user.ts(1 hunks)apps/meteor/client/providers/AuthorizationProvider.tsx(2 hunks)apps/meteor/client/providers/SettingsProvider.tsx(1 hunks)apps/meteor/client/providers/UserProvider/UserProvider.tsx(4 hunks)apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts(1 hunks)apps/meteor/client/startup/accounts.ts(1 hunks)apps/meteor/client/startup/iframeCommands.ts(2 hunks)apps/meteor/client/startup/incomingMessages.ts(2 hunks)apps/meteor/client/startup/roles.ts(2 hunks)apps/meteor/client/startup/startup.ts(1 hunks)apps/meteor/client/stores/Messages.ts(1 hunks)apps/meteor/client/stores/Permissions.ts(1 hunks)apps/meteor/client/stores/Roles.ts(1 hunks)apps/meteor/client/stores/Rooms.ts(1 hunks)apps/meteor/client/stores/Settings.ts(1 hunks)apps/meteor/client/stores/Subscriptions.ts(1 hunks)apps/meteor/client/stores/Users.ts(1 hunks)apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts(1 hunks)apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts(1 hunks)apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts(1 hunks)apps/meteor/client/views/room/MemberListRouter.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts(1 hunks)apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts(1 hunks)apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts(1 hunks)apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts(1 hunks)apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx(1 hunks)apps/meteor/definition/externals/meteor/accounts-base.d.ts(1 hunks)packages/mock-providers/src/MockedAppRootBuilder.tsx(2 hunks)packages/ui-contexts/src/UserContext.ts(2 hunks)packages/ui-contexts/src/hooks/useUserId.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/lib/normalizeThreadTitle.ts
- apps/meteor/client/lib/cachedStores/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/meteor/client/views/room/contextualBar/Threads/components/ThreadListItem.tsx
🚧 Files skipped from review as they are similar to previous changes (32)
- apps/meteor/client/lib/2fa/process2faReturn.ts
- apps/meteor/client/lib/chats/ChatAPI.ts
- apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts
- apps/meteor/client/lib/mutationEffects/starredMessage.ts
- apps/meteor/client/providers/SettingsProvider.tsx
- apps/meteor/client/stores/Subscriptions.ts
- apps/meteor/app/lib/client/methods/sendMessage.ts
- apps/meteor/app/webrtc/client/WebRTCClass.ts
- apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
- apps/meteor/app/autotranslate/client/lib/autotranslate.ts
- apps/meteor/client/lib/user.ts
- apps/meteor/client/startup/incomingMessages.ts
- apps/meteor/client/cachedStores/PrivateSettingsCachedStore.ts
- apps/meteor/client/lib/chats/readStateManager.ts
- apps/meteor/client/lib/cachedStores/CachedStore.ts
- apps/meteor/client/apps/RealAppsEngineUIHost.ts
- apps/meteor/client/lib/utils/timeAgo.ts
- apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
- apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
- apps/meteor/client/cachedStores/RoomsCachedStore.ts
- apps/meteor/client/meteor/user.ts
- apps/meteor/client/stores/Users.ts
- apps/meteor/client/cachedStores/PublicSettingsCachedStore.ts
- apps/meteor/client/meteor/minimongo/DiffSequence.ts
- apps/meteor/client/hooks/roomActions/useMediaCallRoomAction.ts
- apps/meteor/client/lib/e2ee/rocketchat.e2e.ts
- apps/meteor/client/providers/UserProvider/hooks/useClearRemovedRoomsHistory.ts
- apps/meteor/client/views/room/MemberListRouter.tsx
- apps/meteor/client/cachedStores/PermissionsCachedStore.ts
- packages/mock-providers/src/MockedAppRootBuilder.tsx
- apps/meteor/app/ui/client/lib/ChatMessages.ts
- packages/ui-contexts/src/hooks/useUserId.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-01T12:20:13.136Z
Learnt from: sampaiodiego
PR: RocketChat/Rocket.Chat#37110
File: apps/meteor/ee/server/startup/federation.ts:17-21
Timestamp: 2025-10-01T12:20:13.136Z
Learning: In Rocket.Chat's StreamerCentral broadcast for 'notify-room' events with 'user-activity', the `activity` argument is a string (e.g., 'user-typing') when a user starts typing and an empty array when the user stops typing.
Applied to files:
apps/meteor/app/ui/client/lib/UserAction.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
PR: RocketChat/Rocket.Chat#37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/lib/utils/getUidDirectMessage.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
PR: RocketChat/Rocket.Chat#37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/lib/utils/getUidDirectMessage.ts
🧬 Code graph analysis (24)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/lib/chats/flows/processTooLongMessage.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/app/reactions/client/methods/setReaction.ts (1)
apps/meteor/client/lib/user.ts (2)
getUserId(11-11)getUser(13-17)
apps/meteor/client/meteor/overrides/userAndUsers.ts (3)
apps/meteor/tests/mocks/client/meteor.ts (2)
Tracker(44-44)Accounts(39-42)apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/meteor/user.ts (2)
watchUserId(7-7)watchUser(9-13)
apps/meteor/client/lib/getPermaLink.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/lib/chats/data.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/app/authorization/client/hasPermission.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/providers/UserProvider/UserProvider.tsx (4)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)apps/meteor/client/stores/Users.ts (1)
Users(11-13)apps/meteor/client/meteor/minimongo/MinimongoCollection.ts (1)
state(82-84)
apps/meteor/client/startup/iframeCommands.ts (1)
apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/app/otr/client/OTRRoom.ts (2)
apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)apps/meteor/client/lib/user.ts (1)
getUser(13-17)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (3)
packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(306-310)packages/ui-contexts/src/index.ts (2)
useUser(80-80)useSetting(69-69)apps/meteor/app/emoji/client/emojiParser.ts (1)
emojiParser(7-75)
apps/meteor/client/meteor/overrides/ddpOverREST.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/startup/accounts.ts (3)
apps/meteor/client/lib/user.ts (1)
userIdStore(9-9)apps/meteor/client/cachedStores/index.ts (2)
SubscriptionsCachedStore(5-5)PublicSettingsCachedStore(3-3)apps/meteor/client/lib/userData.ts (1)
useUserDataSyncReady(8-8)
apps/meteor/app/utils/client/lib/getUserPreference.ts (2)
apps/meteor/client/startup/iframeCommands.ts (1)
user(80-89)packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/providers/AuthorizationProvider.tsx (2)
apps/meteor/client/lib/createReactiveSubscriptionFactory.ts (1)
createReactiveSubscriptionFactory(7-45)apps/meteor/app/authorization/client/hasRole.ts (1)
hasRole(6-21)
apps/meteor/client/hooks/useIdleConnection.ts (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts (3)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/IUpload.ts (1)
IUpload(3-68)packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/client/lib/loggedIn.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/startup/roles.ts (1)
apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/client/startup/startup.ts (1)
apps/meteor/client/meteor/user.ts (1)
watchUserId(7-7)
apps/meteor/app/ui/client/lib/UserAction.ts (1)
apps/meteor/client/lib/user.ts (2)
getUser(13-17)getUserId(11-11)
apps/meteor/client/lib/utils/getUidDirectMessage.ts (1)
apps/meteor/client/lib/user.ts (1)
getUserId(11-11)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (2)
packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(306-310)apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (1)
useNormalizedThreadTitleHtml(10-49)
🪛 ast-grep (0.39.5)
apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx
[warning] 14-14: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (63)
apps/meteor/definition/externals/meteor/accounts-base.d.ts (1)
79-82: LGTM! Clean typing for Accounts connection.The new
connectionconstant provides proper TypeScript typing for the Accounts connection API with itsuserId()method. Thestring | nullreturn type is correct per the maintainer's confirmation in the previous review discussion.apps/meteor/client/lib/chats/flows/processTooLongMessage.ts (2)
7-7: LGTM! Import added correctly.The import for
getUseris properly added to support the migration away fromMeteor.user().
29-29: LGTM! Migration togetUser()is correct.The replacement of
Meteor.user()?.usernamewithgetUser()?.usernamealigns with the PR's demeteorization objectives. The fallback behavior to'anonymous'is preserved, and the optional chaining remains appropriate sincegetUser()returnsundefinedwhen no user is logged in.apps/meteor/client/startup/startup.ts (2)
10-10: LGTM!The import of
watchUserIdis correctly placed and follows the project's module structure.
17-17: Correct usage ofwatchUserId()in reactive context.The change from
Meteor.userId()towatchUserId()is appropriate here sinceTracker.autorunis a reactive context. The reactivewatchUserId()will correctly trigger re-runs when the user ID changes, maintaining the original behavior while supporting the demeteorization effort.apps/meteor/client/stores/Messages.ts (1)
3-4: LGTM! Import path refactoring aligns with the PR's modularization goals.The split from a barrel import to explicit module imports improves tree-shaking and makes dependencies clearer. No functional changes to the exported store.
apps/meteor/client/stores/Roles.ts (1)
3-4: LGTM! Consistent import refactoring.The explicit import paths align with the broader architectural shift seen across all store modules in this PR. No changes to the public API.
apps/meteor/client/stores/Permissions.ts (1)
3-4: LGTM! Import path refactoring matches the pattern.Explicit imports from DocumentMapStore and createGlobalStore modules maintain consistency with the other store refactors in this PR.
apps/meteor/client/stores/Settings.ts (1)
3-4: LGTM! Import refactoring is consistent.The explicit import paths for both
PublicSettingsandPrivateSettingsfollow the same pattern as the other store modules in this PR.apps/meteor/client/stores/Rooms.ts (1)
3-4: LGTM! Import path refactoring completes the pattern.The explicit imports are consistent with all other store modules in this PR, supporting the modularization and tree-shaking improvements.
apps/meteor/client/meteor/minimongo/common.ts (1)
3-3: LGTM! Import path updated to reflect new utility location.The import path change from
../objectUtilsto../../lib/objectUtilsaligns with the centralization of utility functions.apps/meteor/client/meteor/minimongo/LocalCollection.ts (1)
19-19: LGTM! Type import consolidated to new queries module.The Query type import has been updated to reference
./queries, aligning with the project's type organization strategy.apps/meteor/client/meteor/minimongo/Cursor.ts (1)
13-13: LGTM! Query type imports consolidated to new module.The Query-related type imports have been updated to reference
./queries, maintaining consistency with other minimongo files.apps/meteor/client/meteor/minimongo/MinimongoCollection.ts (1)
4-5: LGTM! Import paths updated to reflect new module organization.Both import path changes align with the project's restructuring:
- Query type import moved to
./queriesfor consistency- createDocumentMapStore centralized to
../../lib/cachedStores/DocumentMapStoreapps/meteor/client/meteor/minimongo/ObserveHandle.ts (1)
4-4: LGTM! Query type import updated for consistency.The Query type import has been updated to reference
./queries, completing the consistent migration across all minimongo files.apps/meteor/client/startup/iframeCommands.ts (2)
13-13: LGTM! Clean import addition.The import follows the existing import organization and correctly brings in the new
getUserhelper.
80-89: LGTM! Correct migration with improved cleanup behavior.The migration from
Meteor.user()togetUser()is implemented correctly. Capturing the user state beforeMeteor.logout()(line 81) ensures the cleanup callbacks (lines 85-87) have access to the user object that was logged in, which is the appropriate behavior for cleanup operations.The simplified guard (line 83) maintains the same logic in a more concise form.
apps/meteor/app/ui/client/lib/UserAction.ts (2)
6-6: LGTM! Import correctly updated for demeteorization.The import of
getUserandgetUserIdfrom the client lib user module correctly replaces the Meteor dependency.
76-84: LGTM! Correct replacement of Meteor.userId() with defensive handling.The handler correctly uses
getUserId()and defensively handles the case whereuidmight be undefined. The early return when the username matches the current user's shown name properly filters out the user's own activities.apps/meteor/client/lib/chats/data.ts (6)
18-18: LGTM!Clean import of the new non-reactive
getUserId()helper as part of the demeteorization effort.
82-82: LGTM!The replacement of
Meteor.userId()withgetUserId()preserves the ownership check logic correctly. Optional chaining safely handles the case wheremessage.uis undefined.
100-104: LGTM!The replacement correctly retrieves the user ID and handles the undefined case with an early return, maintaining the original logic.
138-142: LGTM!Consistent replacement pattern with appropriate undefined handling via early return.
183-187: LGTM!Proper handling of the user ID retrieval with an appropriate early return for the permission check when no user is authenticated.
205-205: LGTM!The ownership check in the delete permission logic correctly uses
getUserId()and maintains the original boolean logic. Optional chaining appropriately guards against undefinedmessage.u.apps/meteor/app/otr/client/OTRRoom.ts (1)
12-12: LGTM: Import aligns with demeteorization effort.The import of
getUsercorrectly replaces the reactiveMeteor.user()with a non-reactive alternative as part of the codebase modernization.apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (1)
29-29: Approve import path refactor. Verified thatapps/meteor/client/lib/cachedStores/pipe.tsexports thepipeutility. No further changes needed.apps/meteor/app/reactions/client/methods/setReaction.ts (3)
6-6: LGTM! Import aligns with demeteorization objectives.The import of
getUserandgetUserIdfrom the centralized user module is correct and supports the PR's goal of replacing direct Meteor global access.
12-14: LGTM! Non-reactive user ID check is appropriate here.The replacement of
Meteor.userId()withgetUserId()is correct. The non-reactive behavior is suitable for this Meteor method context, and the falsy check works correctly regardless of whether the return type isnullorundefined.
16-20: LGTM! User retrieval replacement is safe.The replacement of
Meteor.user()withgetUser()is appropriate. The optional chaining at line 18 correctly handles the possibility ofundefined, and non-reactivity is suitable for this method context.apps/meteor/client/views/navigation/sidepanel/hooks/useChannelsChildrenList.ts (1)
4-4: LGTM: Import path refactored to dedicated module.The import path for
pipehas been updated from the barrel export to a dedicated module, improving module organization with no functional changes.apps/meteor/client/views/admin/permissions/hooks/usePermissionsAndRoles.ts (1)
7-7: LGTM: Import path refactored to dedicated module.The import path for
pipehas been updated from the barrel export to a dedicated module, consistent with the same refactor applied across the codebase.apps/meteor/client/meteor/overrides/userAndUsers.ts (4)
1-7: LGTM: Imports are correct.The new imports support the store-based user resolution approach.
14-14: LGTM: Maintains backward compatibility.The override correctly maintains Meteor.userId()'s
string | nullreturn type by convertingundefinedback tonull, ensuring existing code continues to work.
17-17: LGTM: Type assertion maintains compatibility.The override correctly maintains backward compatibility with Meteor.user()'s return type. The cast from
IUsertoMeteor.Useris necessary for the override but safe sinceIUseris a superset ofMeteor.Userfields.
19-20: LGTM: Type assertion correctly documented.The comment clearly explains the need for the type assertion, and the double-cast pattern is appropriate for overriding with a structurally compatible but nominally different type.
apps/meteor/client/lib/loggedIn.ts (2)
2-3: LGTM: Clean import of the new getUserId helper.The import follows the PR's objective to replace Meteor.userId() with a centralized, non-reactive getUserId() function.
5-8: ApproveisLoggedInchanges; no null/undefined inconsistencies detected.
getUserId()returnsstring | undefined, and there are no=== nullchecks in the codebase—using!!uidcorrectly handles bothundefinedand non-empty strings.apps/meteor/client/meteor/overrides/ddpOverREST.ts (2)
5-5: LGTM! Clean import addition.The import of
getUserIdis correctly added and used on line 35.
35-35: Approve removal of Tracker.nonreactive: getUserId is non-reactivegetUserId uses a zustand store’s getState() and does not establish any Meteor reactive dependencies, so removing the Tracker.nonreactive wrapper is correct.
apps/meteor/client/meteor/overrides/unstoreLoginToken.ts (1)
3-3: Import path resolves correctly
apps/meteor/client/lib/cachedStores/CachedStoresManager.tsexports the singleton instance asCachedStoresManager, matching the updated import.apps/meteor/client/lib/utils/getUidDirectMessage.ts (1)
4-4: LGTM! Import change aligns with demeteorization.The import correctly replaces the Meteor dependency with the new
getUserIdhelper from the centralized user module.apps/meteor/client/lib/mutationEffects/room.ts (1)
3-12: Manual verification required: Confirm thatuseUserId()returnsstring | undefined, ensuring compatibility with the updatedtoggleFavoriteRoomsignature.apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (2)
54-54: Clean pass-through implementation.The
uidparameter is correctly passed through touseStreamUpdatesForMessageListwithout transformation. The implementation correctly relies on type safety rather than runtime checks.
18-18: EnsureuseUserId()return type aligns withuid: IUser['_id'] | undefinedThe hook signature now expects
uidto beIUser['_id'] | undefined. Since all callers (e.g. inContactHistoryMessagesList.tsxandContactInfoHistoryMessages.tsx) passuserId = useUserId(), confirm thatuseUserId()returnsIUser['_id'] | undefined(and notnull), or explicitly coercenulltoundefinedbefore callinguseHistoryMessageList. If it still returnsnull, either updateuseUserId()’s typing/implementation or adjustuseHistoryMessageListand downstreamuseStreamUpdatesForMessageListto acceptnull.apps/meteor/client/providers/UserProvider/UserProvider.tsx (2)
1-1: LGTM: Import changes align with demeteorization.The import reorganization correctly:
- Removes unused
IUserimport- Updates
applyQueryOptionsto direct file import- Adds
userIdStorefor centralized user ID managementAlso applies to: 25-25, 27-27
68-74: LGTM: Correct store-based user retrieval.The component correctly uses
userIdStore()(imported from../../lib/user) to obtain the current user ID and derives the user object viaUsers.use. This aligns with the PR's objective to replaceMeteor.userId()with a centralized store-based approach.Addresses the past comment questioning store usage:
userIdStoreis indeed fromapps/meteor/client/lib/user.tsas shown in line 27.apps/meteor/client/lib/getPermaLink.ts (2)
2-3: LGTM! Clean import replacement.The import change correctly replaces the Meteor dependency with the new getUserId helper from the local user module.
32-32: Fallback for unauthenticated users is intentional
getUserId()returnsundefinedby default, sosubDatacorrectly falls back toroomData.apps/meteor/client/startup/roles.ts (2)
7-7: LGTM! Import added for demeteorization.The import of
watchUserIdaligns with the PR objective to replaceMeteor.userId()with a centralized user store helper.
29-39: No action needed:watchUserIdintegrates with Tracker reactivity.
Thewatchhelper usesTracker.currentComputationto subscribe to the ZustanduserIdStoreand invalidates the computation on changes, so the autorun will rerun when the user logs in and correctly establish the roles stream.apps/meteor/client/views/room/contextualBar/Threads/components/ThreadTitle.tsx (1)
5-14: LGTM: Hook-based refactor improves separation of concerns.This addresses the previous feedback about isolating the normalization logic. The hook pattern provides better testability and keeps the component focused on rendering.
The
dangerouslySetInnerHTMLwarning is addressed by the hook's sanitization logic (reviewed separately).apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts (2)
35-45: LGTM: Attachment handling is correctly escaped.The attachment title and description are properly escaped with
escapeHTMLbefore returning, preventing XSS vulnerabilities.
19-19: Fix escaping order to prevent markdown processing issues.Escaping HTML before filtering markdown will prevent markdown syntax from being processed correctly and can lead to double-escaping. The markdown filter should operate on raw text, then the output should be escaped.
Apply this diff:
- const filteredMessage = filterMarkdown(escapeHTML(message.msg)); + const filteredMessage = escapeHTML(filterMarkdown(message.msg));Note: This issue was previously flagged and marked as addressed but the fix was not applied correctly.
Likely an incorrect or invalid review comment.
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts (1)
7-7: Confirmed nonullcall sites remain.useMessageDeletionIsAllowedis only invoked withuseUserId(), which returnsstring | undefined, so the signature change is fully compatible.packages/ui-contexts/src/UserContext.ts (1)
32-32: I’ve provided a script to inspectgetUserId’s implementation for its return type. Let me know the output so we can confirm alignment with theUserContextchange.apps/meteor/client/hooks/useIdleConnection.ts (2)
1-1: LGTM! Type signature updated consistently.The import of
IUserand the parameter type change fromstring | nulltoIUser['_id'] | undefinedcorrectly align with the PR's goal of standardizing user ID types and removingnullin favor ofundefined.Also applies to: 7-7
14-14: Existing logic handlesundefinedcorrectly.The falsy check
!uidworks identically for bothnullandundefined, so the type change does not impact runtime behavior.apps/meteor/app/authorization/client/hasPermission.ts (3)
5-5: LGTM! Replaces Meteor.userId() with watchUserId() and aligns types.The import of
watchUserIdand the updated parameter types (removing| nullin favor of optionalundefined) correctly implement the PR's demeteorization goal and type standardization.Also applies to: 50-50, 78-78
63-82: Core permission evaluation logic unchanged.The predicate application and exported functions retain their original logic. Only the user ID sourcing and readiness gating have been updated.
59-61: Defer permission checks untilPermissionsCachedStore.watchReady()resolves
In audit.tsx (hasAllPermission at line 34) and autotranslate.ts (hasPermission at line 10), the newwatchReady()gate causes checks to return false—rendering<NotAuthorizedPage/>or removing callbacks—before the store hydrates. Introduce a loading state or delay these checks untilwatchReady()is true, and verify the readiness flag fires during startup.apps/meteor/app/utils/client/lib/getUserPreference.ts (2)
12-12: LGTM! String ID overloads updated to useundefined.The parameter type change from
IUser['_id'] | nulltoIUser['_id'] | undefinedaligns with the PR's standardization effort.Also applies to: 27-27
40-47: Implementation correctly handles both string ID and user object.The type guard at line 45 and optional chaining at line 46 correctly differentiate between string IDs and user objects, with proper fallback to
defaultValueand global settings.
apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.ts
Show resolved
Hide resolved
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
It introduces
getUserId()as a non-reactive replacement forMeteor.userId().Issue(s)
Steps to test or reproduce
Further comments
This PR is part of our “demeteorization” initiative.
It's a slice of #36858.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores